[Lldb-commits] [PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

2021-02-27 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa54f160b3a98: Prefer /usr/bin/env xxx over /usr/bin/xxx 
where xxx = perl, python, awk (authored by haampie, committed by teemperor).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95119/new/

https://reviews.llvm.org/D95119

Files:
  clang/test/make_test_dirs.pl
  clang/tools/scan-build/bin/set-xcode-analyzer
  clang/utils/TestUtils/pch-test.pl
  clang/utils/analyzer/reducer.pl
  clang/utils/analyzer/update_plist_test.pl
  clang/www/demo/index.cgi
  debuginfo-tests/llgdb-tests/test_debuginfo.pl
  lldb/docs/use/python-reference.rst
  lldb/scripts/disasm-gdb-remote.pl
  llvm/utils/GenLibDeps.pl
  llvm/utils/codegen-diff
  llvm/utils/findsym.pl
  llvm/utils/llvm-compilers-check
  llvm/utils/llvm-native-gxx
  openmp/runtime/tools/check-execstack.pl
  openmp/runtime/tools/check-instruction-set.pl
  openmp/runtime/tools/message-converter.pl
  polly/lib/External/isl/doc/mypod2latex

Index: polly/lib/External/isl/doc/mypod2latex
===
--- polly/lib/External/isl/doc/mypod2latex
+++ polly/lib/External/isl/doc/mypod2latex
@@ -1,4 +1,4 @@
-#!/usr/bin/perl
+#!/usr/bin/env perl
 
 use strict;
 use Pod::LaTeX;
Index: openmp/runtime/tools/message-converter.pl
===
--- openmp/runtime/tools/message-converter.pl
+++ openmp/runtime/tools/message-converter.pl
@@ -1,4 +1,4 @@
-#!/usr/bin/perl
+#!/usr/bin/env perl
 
 #
 #//===--===//
Index: openmp/runtime/tools/check-instruction-set.pl
===
--- openmp/runtime/tools/check-instruction-set.pl
+++ openmp/runtime/tools/check-instruction-set.pl
@@ -1,4 +1,4 @@
-#!/usr/bin/perl
+#!/usr/bin/env perl
 
 #
 #//===--===//
Index: openmp/runtime/tools/check-execstack.pl
===
--- openmp/runtime/tools/check-execstack.pl
+++ openmp/runtime/tools/check-execstack.pl
@@ -1,4 +1,4 @@
-#!/usr/bin/perl
+#!/usr/bin/env perl
 
 #
 #//===--===//
Index: llvm/utils/llvm-native-gxx
===
--- llvm/utils/llvm-native-gxx
+++ llvm/utils/llvm-native-gxx
@@ -1,4 +1,4 @@
-#!/usr/bin/perl
+#!/usr/bin/env perl
 # Wrapper around LLVM tools to generate a native .o from llvm-gxx using an
 # LLVM back-end (CBE by default).
 
Index: llvm/utils/llvm-compilers-check
===
--- llvm/utils/llvm-compilers-check
+++ llvm/utils/llvm-compilers-check
@@ -1,4 +1,4 @@
-#!/usr/bin/python3
+#!/usr/bin/env python3
 ##===- utils/llvmbuild - Build the LLVM project *-python-*-===##
 #
 # Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
Index: llvm/utils/findsym.pl
===
--- llvm/utils/findsym.pl
+++ llvm/utils/findsym.pl
@@ -1,4 +1,4 @@
-#!/usr/bin/perl -w
+#!/usr/bin/env perl
 #
 # Program:  findsym.pl
 #
@@ -8,6 +8,8 @@
 # Syntax:   findsym.pl  
 #
 
+use warnings;
+
 # Give first option a name.
 my $Directory = $ARGV[0];
 my $Symbol = $ARGV[1];
Index: llvm/utils/codegen-diff
===
--- llvm/utils/codegen-diff
+++ llvm/utils/codegen-diff
@@ -1,4 +1,4 @@
-#!/usr/bin/perl
+#!/usr/bin/env perl
 
 use Getopt::Std;
 $DEBUG = 0;
Index: llvm/utils/GenLibDeps.pl
===
--- llvm/utils/GenLibDeps.pl
+++ llvm/utils/GenLibDeps.pl
@@ -1,4 +1,4 @@
-#!/usr/bin/perl -w
+#!/usr/bin/env perl
 #
 # Program:  GenLibDeps.pl
 #
Index: lldb/scripts/disasm-gdb-remote.pl
===
--- lldb/scripts/disasm-gdb-remote.pl
+++ lldb/scripts/disasm-gdb-remote.pl
@@ -1,4 +1,4 @@
-#!/usr/bin/perl
+#!/usr/bin/env perl
 
 use strict;
 
Index: lldb/docs/use/python-reference.rst
===
--- lldb/docs/use/python-reference.rst
+++ lldb/docs/use/python-reference.rst
@@ -607,7 +607,7 @@
 
 ::
 
-  #!/usr/bin/python
+  #!/usr/bin/env python
 
   import lldb
   import commands
@@ -715,7 +715,7 @@
 
 ::
 
-  #!/usr/bin/python
+  #!/usr/bin/env python
 
   import lldb
   import os
Index: debuginfo-tests/llgdb-tests/test_debuginfo.pl
===
--- debuginfo-tests/llgdb-tests/test_debuginfo.pl
+++ debuginfo-tests/llgdb-tests/test_debuginfo.pl
@@ -1,4 +1,4 @@
-#!/usr/bin/perl
+#!/usr/bin/env perl
 #
 # This script tests debugging information generated by a compiler.
 # Input argum

[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-27 Thread Reid Kleckner via Phabricator via lldb-commits
rnk added a comment.

This still feels a bit messy and I don't totally understand how it all fits 
together, but I'm super supportive of getting rid of the inline asm diagnostic 
handler and standardizing on DiagnosticHandler/DiagnosticInfo.




Comment at: llvm/include/llvm/MC/MCContext.h:33
 #include "llvm/Support/MD5.h"
+#include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"

MCContext.h is popular, let's try not to leak SourceMgr.h as an include. It 
probably brings in tons of FS headers that most files don't need.



Comment at: llvm/include/llvm/MC/MCContext.h:76
+std::function &)>;
 

It doesn't feel right to use MDNode, an IR type, from MC. Is there a way to 
make this opaque?



Comment at: llvm/include/llvm/MC/MCContext.h:386
+  if (!InlineSrcMgr)
+InlineSrcMgr.reset(new SourceMgr());
+}

This would need to be out of line to get by with a forward decl of SourceMgr.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97449/new/

https://reviews.llvm.org/D97449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-27 Thread Yuanfang Chen via Phabricator via lldb-commits
ychen updated this revision to Diff 326797.
ychen added a comment.

- Simplify MCContext changes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97449/new/

https://reviews.llvm.org/D97449

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/MC/MCContext.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
  llvm/lib/CodeGen/MachineModuleInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/test/CodeGen/AMDGPU/lds-initializer.ll
  llvm/test/CodeGen/AMDGPU/lds-zero-initializer.ll
  llvm/test/CodeGen/XCore/section-name.ll
  llvm/tools/llc/llc.cpp

Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -290,6 +290,22 @@
   bool *HasError;
   LLCDiagnosticHandler(bool *HasErrorPtr) : HasError(HasErrorPtr) {}
   bool handleDiagnostics(const DiagnosticInfo &DI) override {
+if (DI.getKind() == llvm::DK_SrcMgr) {
+  const auto &DISM = cast(DI);
+  const SMDiagnostic &SMD = DISM.getSMDiag();
+
+  if (SMD.getKind() == SourceMgr::DK_Error)
+*HasError = true;
+
+  SMD.print(nullptr, errs());
+
+  // For testing purposes, we print the LocCookie here.
+  if (DISM.isInlineAsmDiag() && DISM.getLocCookie())
+WithColor::note() << "!srcloc = " << DISM.getLocCookie() << "\n";
+
+  return true;
+}
+
 if (DI.getSeverity() == DS_Error)
   *HasError = true;
 
@@ -305,19 +321,6 @@
   }
 };
 
-static void InlineAsmDiagHandler(const SMDiagnostic &SMD, void *Context,
- unsigned LocCookie) {
-  bool *HasError = static_cast(Context);
-  if (SMD.getKind() == SourceMgr::DK_Error)
-*HasError = true;
-
-  SMD.print(nullptr, errs());
-
-  // For testing purposes, we print the LocCookie here.
-  if (LocCookie)
-WithColor::note() << "!srcloc = " << LocCookie << "\n";
-}
-
 // main - Entry point for the llc compiler.
 //
 int main(int argc, char **argv) {
@@ -367,7 +370,6 @@
   bool HasError = false;
   Context.setDiagnosticHandler(
   std::make_unique(&HasError));
-  Context.setInlineAsmDiagnosticHandler(InlineAsmDiagHandler, &HasError);
 
   Expected> RemarksFileOrErr =
   setupLLVMOptimizationRemarks(Context, RemarksFilename, RemarksPasses,
Index: llvm/test/CodeGen/XCore/section-name.ll
===
--- llvm/test/CodeGen/XCore/section-name.ll
+++ llvm/test/CodeGen/XCore/section-name.ll
@@ -1,4 +1,4 @@
-; RUN: llc < %s -march=xcore -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc < %s -march=xcore -o /dev/null 2>&1 | FileCheck %s
 
 @bar = internal global i32 zeroinitializer
 
Index: llvm/test/CodeGen/AMDGPU/lds-zero-initializer.ll
===
--- llvm/test/CodeGen/AMDGPU/lds-zero-initializer.ll
+++ llvm/test/CodeGen/AMDGPU/lds-zero-initializer.ll
@@ -1,5 +1,5 @@
-; RUN: llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
-; RUN: llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
 
 ; CHECK: lds: unsupported initializer for address space
 
Index: llvm/test/CodeGen/AMDGPU/lds-initializer.ll
===
--- llvm/test/CodeGen/AMDGPU/lds-initializer.ll
+++ llvm/test/CodeGen/AMDGPU/lds-initializer.ll
@@ -1,5 +1,5 @@
-; RUN: llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
-; RUN: llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
 
 ; CHECK: lds: unsupported initializer for address space
 
Index: llvm/lib/MC/MCParser/AsmParser.cpp
===
--- llvm/lib/MC/MCParser/AsmParser.cpp
+++ llvm/lib/MC/MCParser/AsmParser.cpp
@@ -2348,7 +2348,7 @@
 /// will use the last parsed cpp hash line filename comment
 /// for the Filename and LineNo if any in the diagnostic.
 void AsmParser::DiagHandler(const SMDiagnostic &Diag, void *Context) {
-  const AsmParser *Parser = static_cast(Context);
+  auto *Parser = static_cast(Context);
   raw_ostream &OS = errs();
 
   const SourceMgr &DiagSrcMgr = *Diag.getSourceMgr();
@@ -2369,12 +2369,8 @@
   // If we have not parsed a cpp hash line filename comment or the source
   // manager changed or buffer changed (like in a nested include) then just
   // print the normal diagnostic using its Filename an

[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-27 Thread Yuanfang Chen via Phabricator via lldb-commits
ychen updated this revision to Diff 326597.
ychen added a comment.

- Refrain from including SourceMgr.h in MCContext.h


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97449/new/

https://reviews.llvm.org/D97449

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/MC/MCContext.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
  llvm/lib/CodeGen/MachineModuleInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/test/CodeGen/AMDGPU/lds-initializer.ll
  llvm/test/CodeGen/AMDGPU/lds-zero-initializer.ll
  llvm/test/CodeGen/XCore/section-name.ll
  llvm/tools/llc/llc.cpp

Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -290,6 +290,22 @@
   bool *HasError;
   LLCDiagnosticHandler(bool *HasErrorPtr) : HasError(HasErrorPtr) {}
   bool handleDiagnostics(const DiagnosticInfo &DI) override {
+if (DI.getKind() == llvm::DK_SrcMgr) {
+  const auto &DISM = cast(DI);
+  const SMDiagnostic &SMD = DISM.getSMDiag();
+
+  if (SMD.getKind() == SourceMgr::DK_Error)
+*HasError = true;
+
+  SMD.print(nullptr, errs());
+
+  // For testing purposes, we print the LocCookie here.
+  if (DISM.isInlineAsmDiag() && DISM.getLocCookie())
+WithColor::note() << "!srcloc = " << DISM.getLocCookie() << "\n";
+
+  return true;
+}
+
 if (DI.getSeverity() == DS_Error)
   *HasError = true;
 
@@ -305,19 +321,6 @@
   }
 };
 
-static void InlineAsmDiagHandler(const SMDiagnostic &SMD, void *Context,
- unsigned LocCookie) {
-  bool *HasError = static_cast(Context);
-  if (SMD.getKind() == SourceMgr::DK_Error)
-*HasError = true;
-
-  SMD.print(nullptr, errs());
-
-  // For testing purposes, we print the LocCookie here.
-  if (LocCookie)
-WithColor::note() << "!srcloc = " << LocCookie << "\n";
-}
-
 // main - Entry point for the llc compiler.
 //
 int main(int argc, char **argv) {
@@ -367,7 +370,6 @@
   bool HasError = false;
   Context.setDiagnosticHandler(
   std::make_unique(&HasError));
-  Context.setInlineAsmDiagnosticHandler(InlineAsmDiagHandler, &HasError);
 
   Expected> RemarksFileOrErr =
   setupLLVMOptimizationRemarks(Context, RemarksFilename, RemarksPasses,
Index: llvm/test/CodeGen/XCore/section-name.ll
===
--- llvm/test/CodeGen/XCore/section-name.ll
+++ llvm/test/CodeGen/XCore/section-name.ll
@@ -1,4 +1,4 @@
-; RUN: llc < %s -march=xcore -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc < %s -march=xcore -o /dev/null 2>&1 | FileCheck %s
 
 @bar = internal global i32 zeroinitializer
 
Index: llvm/test/CodeGen/AMDGPU/lds-zero-initializer.ll
===
--- llvm/test/CodeGen/AMDGPU/lds-zero-initializer.ll
+++ llvm/test/CodeGen/AMDGPU/lds-zero-initializer.ll
@@ -1,5 +1,5 @@
-; RUN: llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
-; RUN: llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
 
 ; CHECK: lds: unsupported initializer for address space
 
Index: llvm/test/CodeGen/AMDGPU/lds-initializer.ll
===
--- llvm/test/CodeGen/AMDGPU/lds-initializer.ll
+++ llvm/test/CodeGen/AMDGPU/lds-initializer.ll
@@ -1,5 +1,5 @@
-; RUN: llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
-; RUN: llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
 
 ; CHECK: lds: unsupported initializer for address space
 
Index: llvm/lib/MC/MCParser/AsmParser.cpp
===
--- llvm/lib/MC/MCParser/AsmParser.cpp
+++ llvm/lib/MC/MCParser/AsmParser.cpp
@@ -54,6 +54,7 @@
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SMLoc.h"
+#include "llvm/Support/SaveAndRestore.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -892,6 +893,8 @@
 }
 
 bool AsmParser::Run(bool NoInitialTextSection, bool NoFinalize) {
+  SaveAndRestore TurnOnAsmParseDiag(Ctx.AsmParseMode, true);
+
   // Create the initial section, if requested.
   if (!NoInitialTextSection)
 Out.InitSections(false);
@@ -2348,7 +2351,7 @@
 /// will use the last parsed cpp hash line filename co

[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-27 Thread Yuanfang Chen via Phabricator via lldb-commits
ychen added inline comments.



Comment at: llvm/include/llvm/MC/MCContext.h:33
 #include "llvm/Support/MD5.h"
+#include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"

rnk wrote:
> MCContext.h is popular, let's try not to leak SourceMgr.h as an include. It 
> probably brings in tons of FS headers that most files don't need.
will do


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97449/new/

https://reviews.llvm.org/D97449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-27 Thread Yuanfang Chen via Phabricator via lldb-commits
ychen added a comment.

In D97449#2588804 , @rnk wrote:

> This still feels a bit messy and I don't totally understand how it all fits 
> together, but I'm super supportive of getting rid of the inline asm 
> diagnostic handler and standardizing on DiagnosticHandler/DiagnosticInfo.

Fully agree. It took me a while to collect (hopefully) all the pieces. :-)

I think part of the complexity stems from the factor that frontend clients like 
Clang owns the SourceMgr equivalent and handles the source code mapping part 
(LLVMContext doesn't control source managers), but LLVM internally 
(MC/MCContext) uses SourceMgr for scenarios like reporting assembly related 
errors (inlineasem, or input asm file). So in 
AsmParser/AsmPrinter/AsmPrinterInlineAsm, there is code about SourceMgr and its 
handlers here and there.

There are two improvements that could be done for the next step:

- Move SourceMgr related processing in AsmParser/AsmPrinter/AsmPrinterInlineAsm 
to MCContext.
- Merge MCContext::SrcMgr and MCContext::InlineSrcMgr because in all use cases, 
only one of them is used.

Both make MCContext the main class for MC reporting and simplify things.

It may help to go through this patch in this order:

- DiagnosticInfo.h (introduces new diag info kind)
- AsmPrinterInlineAsm.cpp, MachineModuleInfo.cpp ([1] make MCContext own the 
information for inlineasm reporting [2]hook LLVMContext's handler to 
MCContext's reporting methods)
- LLVMContext.cpp/LLVMContextImpl.cpp (kill LLVMContext inlineasm handler)
- Clang/lldb/llc (switch clients to use DiagnosticInfoSrcMgr)
- AsmParser.cpp (this is a little bit tricky but it just switches to use the 
handler passed from LLVMContext to MCContext.)




Comment at: llvm/include/llvm/MC/MCContext.h:76
+std::function &)>;
 

rnk wrote:
> It doesn't feel right to use MDNode, an IR type, from MC. Is there a way to 
> make this opaque?
Yeah, I've paid attention to that. Here MCContext owns the std::vector but 
doesn't need to do any processing with it. Forward declaring `MDNode` is all it 
needs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97449/new/

https://reviews.llvm.org/D97449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D97586: [mlir][lldb] Fix several gcc warnings in mlir and lldb

2021-02-27 Thread Stella Stamenova via Phabricator via lldb-commits
stella.stamenova created this revision.
Herald added subscribers: cota, teijeong, rdzhabarov, tatianashp, ThomasRaoux, 
AlexeySotkin, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, 
aartbik, lucyrfox, mgester, arpith-jacob, antiagainst, shauheen, rriddle, 
mehdi_amini.
Herald added a reviewer: mravishankar.
Herald added a reviewer: antiagainst.
Herald added a reviewer: aartbik.
stella.stamenova requested review of this revision.
Herald added subscribers: lldb-commits, stephenneuendorffer, nicolasvasilache.
Herald added projects: LLDB, MLIR.

These warnings are raised when compiling with gcc due to either having too few 
or too many commas, or in the case of lldb, the possibility of a nullptr.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97586

Files:
  lldb/source/Commands/CommandObjectTrace.cpp
  mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp
  mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp


Index: mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp
===
--- mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp
+++ mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp
@@ -119,12 +119,12 @@
 // Mapping traits.
 
//===--===//
 
-LLVM_YAML_IS_SEQUENCE_VECTOR(LinalgTensorDef);
-LLVM_YAML_IS_SEQUENCE_VECTOR(SerializedAffineMap);
-LLVM_YAML_IS_SEQUENCE_VECTOR(LinalgIteratorTypeDef);
-LLVM_YAML_IS_SEQUENCE_VECTOR(ScalarAssign);
-LLVM_YAML_IS_SEQUENCE_VECTOR(ScalarExpression);
-LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(LinalgOpConfig);
+LLVM_YAML_IS_SEQUENCE_VECTOR(LinalgTensorDef)
+LLVM_YAML_IS_SEQUENCE_VECTOR(SerializedAffineMap)
+LLVM_YAML_IS_SEQUENCE_VECTOR(LinalgIteratorTypeDef)
+LLVM_YAML_IS_SEQUENCE_VECTOR(ScalarAssign)
+LLVM_YAML_IS_SEQUENCE_VECTOR(ScalarExpression)
+LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(LinalgOpConfig)
 
 namespace llvm {
 namespace yaml {
Index: mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp
===
--- mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp
+++ mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp
@@ -27,7 +27,7 @@
 /// attribute.
 static uint64_t getFirstIntValue(ArrayAttr attr) {
   return (*attr.getAsValueRange().begin()).getZExtValue();
-};
+}
 
 namespace {
 
Index: lldb/source/Commands/CommandObjectTrace.cpp
===
--- lldb/source/Commands/CommandObjectTrace.cpp
+++ lldb/source/Commands/CommandObjectTrace.cpp
@@ -116,7 +116,7 @@
 Trace::FindPlugin(GetDebugger(), *session_file,
   json_file.GetDirectory().AsCString())) {
   lldb::TraceSP trace_sp = traceOrErr.get();
-  if (m_options.m_verbose)
+  if (m_options.m_verbose && trace_sp)
 result.AppendMessageWithFormat("loading trace with plugin %s\n",
trace_sp->GetPluginName().AsCString());
 } else


Index: mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp
===
--- mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp
+++ mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp
@@ -119,12 +119,12 @@
 // Mapping traits.
 //===--===//
 
-LLVM_YAML_IS_SEQUENCE_VECTOR(LinalgTensorDef);
-LLVM_YAML_IS_SEQUENCE_VECTOR(SerializedAffineMap);
-LLVM_YAML_IS_SEQUENCE_VECTOR(LinalgIteratorTypeDef);
-LLVM_YAML_IS_SEQUENCE_VECTOR(ScalarAssign);
-LLVM_YAML_IS_SEQUENCE_VECTOR(ScalarExpression);
-LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(LinalgOpConfig);
+LLVM_YAML_IS_SEQUENCE_VECTOR(LinalgTensorDef)
+LLVM_YAML_IS_SEQUENCE_VECTOR(SerializedAffineMap)
+LLVM_YAML_IS_SEQUENCE_VECTOR(LinalgIteratorTypeDef)
+LLVM_YAML_IS_SEQUENCE_VECTOR(ScalarAssign)
+LLVM_YAML_IS_SEQUENCE_VECTOR(ScalarExpression)
+LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(LinalgOpConfig)
 
 namespace llvm {
 namespace yaml {
Index: mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp
===
--- mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp
+++ mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp
@@ -27,7 +27,7 @@
 /// attribute.
 static uint64_t getFirstIntValue(ArrayAttr attr) {
   return (*attr.getAsValueRange().begin()).getZExtValue();
-};
+}
 
 namespace {
 
Index: lldb/source/Commands/CommandObjectTrace.cpp
===
--- lldb/source/Commands/CommandObjectTrace.cpp
+++ lldb/source/Commands/CommandObjectTrace.cpp
@@ -116,7 +116,7 @@
 Trace::FindPlugin(GetDebugger(), *session_file,
   json_file.GetDirectory().AsCString())) {
   lldb::TraceSP trace_sp = traceOrErr.get();
-  if (m_options.m_verbose)
+  if (m_options.m_verbose && trace_sp)
 result.AppendMe

[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-27 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment.

I am supportive of getting rid of InlineAsmDiagnosticHandler, too.

The updated AMDGPU tests suggeste that previously `MCContext::reportError` may 
be called with no `SrcMgr` or `InlineSrcMgr`, so the error is propagated to the 
temporary `SourceMgr()`. The `LLCDiagnosticHandler` does not receive anything 
so the exit code is incorrect 0.

The new behavior moves the `SrcMgr` or `InlineSrcMgr` check under `if 
(Loc.isValid()) {` (in `MCContext::reportCommon`). There is still a temporary 
`SrcMgr`. I wonder whether this can be improved.




Comment at: llvm/include/llvm/IR/DiagnosticInfo.h:1036
+  void print(DiagnosticPrinter &DP) const override {
+llvm_unreachable("unimplemented");
+  }

@nickdesaulniers's diagnostic means this is reachable.

Perhaps `Diagnostic.print` needs to be called with appropriate arguments.



Comment at: llvm/lib/MC/MCContext.cpp:869
+SMLoc Loc,
+std::function GetMessage) {
+  SourceMgr SM;

Use lightweight function_ref since you don't need to store the callback.



Comment at: llvm/lib/MC/MCContext.cpp:870
+std::function GetMessage) {
+  SourceMgr SM;
+  const SourceMgr *SMP = &SM;

This looks a bit strange: we need to construct a fresh SourceMgr to print a 
diagnostic.



Comment at: llvm/test/CodeGen/AMDGPU/lds-initializer.ll:1-2
-; RUN: llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
-; RUN: llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
 

nickdesaulniers wrote:
> Does the addition of `not` mean that this test would have failed due to these 
> changes? How come?
This is an improvement. Errors should return non-zero. It might be clear to 
change the CHECK below to have `error:`.

In the new code, `LLCDiagnosticHandler` sets `HasError` to true.



Comment at: llvm/test/CodeGen/AMDGPU/lds-zero-initializer.ll:1
-; RUN: llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
-; RUN: llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s

While here, `-o /dev/null` -> `-filetype=null`



Comment at: llvm/test/CodeGen/AMDGPU/lds-zero-initializer.ll:4
 
 ; CHECK: lds: unsupported initializer for address space
 

Add `error:` to make it clear this is an error.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97449/new/

https://reviews.llvm.org/D97449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-27 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:477
+  StringRef Message = D.getMessage();
+  if (Message.startswith("error: "))
+Message = Message.substr(7);

`StringRef::consume_front`

I know you are moving code, but do you know why it needs to chop off the 
`error: ` prefix (why does the message get a prefix here?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97449/new/

https://reviews.llvm.org/D97449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-27 Thread Nick Desaulniers via Phabricator via lldb-commits
nickdesaulniers added a comment.

I tested this on some wacko LTO kernel build failure 
(https://github.com/ClangBuiltLinux/linux/issues/1269).  The error message went 
from:

> :0: error: __ia32_compat_sys_sysctl changed binding to STB_GLOBAL

To:

  unimplemented
  UNREACHABLE executed at ../include/llvm/IR/DiagnosticInfo.h:1036!
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace.
  Stack dump:
  0.  Program arguments: ld.lld -m elf_x86_64 -z max-page-size=0x20 
-plugin-opt=-code-model=kernel -plugin-opt=-stack-alignment=8 -mllvm 
-import-instr-limit=5 -r -o vmlinux.o -T .tmp_lto.lds --whole-archive 
built-in.a --no-whole-archive --start-group lib/lib.a arch/x86/lib/lib.a 
--end-group
  1.  Running pass 'Function Pass Manager' on module 'ld-temp.o'.
  2.  Running pass 'X86 Assembly Printer' on function 
'@__ia32_compat_sys_sysctl'
   #0 0x01bb2d13 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/android0/llvm-project/llvm/build/bin/lld+0x1bb2d13)
   #1 0x01bb0a5e llvm::sys::RunSignalHandlers() 
(/android0/llvm-project/llvm/build/bin/lld+0x1bb0a5e)
   #2 0x01bb330f SignalHandler(int) Signals.cpp:0:0
   #3 0x7f5d1768b140 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x14140)
   #4 0x7f5d16fb5ce1 raise ./signal/../sysdeps/unix/sysv/linux/raise.c:51:1
   #5 0x7f5d16f9f537 abort ./stdlib/abort.c:81:7
   #6 0x01b2a261 (/android0/llvm-project/llvm/build/bin/lld+0x1b2a261)
   #7 0x02cd3199 (/android0/llvm-project/llvm/build/bin/lld+0x2cd3199)
   #8 0x01bb4479 lld::diagnosticHandler(llvm::DiagnosticInfo const&) 
(/android0/llvm-project/llvm/build/bin/lld+0x1bb4479)
   #9 0x02bb03bf 
llvm::lto::LTOLLVMDiagnosticHandler::handleDiagnostics(llvm::DiagnosticInfo 
const&) LTO.cpp:0:0
  #10 0x0407f96c llvm::LLVMContext::diagnose(llvm::DiagnosticInfo 
const&) (/android0/llvm-project/llvm/build/bin/lld+0x407f96c)
  #11 0x02cd30b4 std::_Function_handler >&), 
llvm::MachineModuleInfoWrapperPass::doInitialization(llvm::Module&)::$_1>::_M_invoke(std::_Any_data
 const&, llvm::SMDiagnostic const&, bool&&, llvm::SourceMgr const&, 
std::vector >&) 
MachineModuleInfo.cpp:0:0
  #12 0x03e5e87a llvm::MCContext::reportCommon(llvm::SMLoc, 
std::function) 
(/android0/llvm-project/llvm/build/bin/lld+0x3e5e87a)
  #13 0x03e5aeee llvm::MCContext::reportError(llvm::SMLoc, llvm::Twine 
const&) (/android0/llvm-project/llvm/build/bin/lld+0x3e5aeee)
  #14 0x03e6c7de 
llvm::MCELFStreamer::emitSymbolAttribute(llvm::MCSymbol*, llvm::MCSymbolAttr) 
(/android0/llvm-project/llvm/build/bin/lld+0x3e6c7de)
  #15 0x0271b6f2 llvm::AsmPrinter::emitFunctionHeader() 
(/android0/llvm-project/llvm/build/bin/lld+0x271b6f2)
  #16 0x0271cb3d llvm::AsmPrinter::emitFunctionBody() 
(/android0/llvm-project/llvm/build/bin/lld+0x271cb3d)
  #17 0x02482556 
llvm::X86AsmPrinter::runOnMachineFunction(llvm::MachineFunction&) 
X86AsmPrinter.cpp:0:0
  #18 0x02cb37fe 
llvm::MachineFunctionPass::runOnFunction(llvm::Function&) 
(/android0/llvm-project/llvm/build/bin/lld+0x2cb37fe)
  #19 0x040912a8 llvm::FPPassManager::runOnFunction(llvm::Function&) 
(/android0/llvm-project/llvm/build/bin/lld+0x40912a8)
  #20 0x040979a8 llvm::FPPassManager::runOnModule(llvm::Module&) 
(/android0/llvm-project/llvm/build/bin/lld+0x40979a8)
  #21 0x04091957 llvm::legacy::PassManagerImpl::run(llvm::Module&) 
(/android0/llvm-project/llvm/build/bin/lld+0x4091957)




Comment at: llvm/test/CodeGen/AMDGPU/lds-initializer.ll:1-2
-; RUN: llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
-; RUN: llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
 

Does the addition of `not` mean that this test would have failed due to these 
changes? How come?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97449/new/

https://reviews.llvm.org/D97449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D50299: Migrate to llvm::unique_function instead of static member functions for callbacks

2021-02-27 Thread Neal via Phabricator via lldb-commits
nealsid updated this revision to Diff 326954.
nealsid removed a project: LLDB.
nealsid added a comment.

Updated diff with run of clang-format and added a check for function pointer 
validity.  Thanks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D50299/new/

https://reviews.llvm.org/D50299

Files:
  lldb/include/lldb/Core/IOHandler.h
  lldb/include/lldb/Host/Editline.h
  lldb/source/Core/IOHandler.cpp
  lldb/source/Host/common/Editline.cpp
  lldb/unittests/Editline/EditlineTest.cpp

Index: lldb/unittests/Editline/EditlineTest.cpp
===
--- lldb/unittests/Editline/EditlineTest.cpp
+++ lldb/unittests/Editline/EditlineTest.cpp
@@ -81,8 +81,8 @@
   void ConsumeAllOutput();
 
 private:
-  static bool IsInputComplete(lldb_private::Editline *editline,
-  lldb_private::StringList &lines, void *baton);
+  bool IsInputComplete(lldb_private::Editline *editline,
+   lldb_private::StringList &lines);
 
   std::unique_ptr _editline_sp;
 
@@ -122,7 +122,10 @@
   _editline_sp->SetPrompt("> ");
 
   // Hookup our input complete callback.
-  _editline_sp->SetIsInputCompleteCallback(IsInputComplete, this);
+  auto input_complete_cb = [this](Editline *editline, StringList &lines) {
+return this->IsInputComplete(editline, lines);
+  };
+  _editline_sp->SetIsInputCompleteCallback(input_complete_cb);
 }
 
 void EditlineAdapter::CloseInput() {
@@ -183,8 +186,7 @@
 }
 
 bool EditlineAdapter::IsInputComplete(lldb_private::Editline *editline,
-  lldb_private::StringList &lines,
-  void *baton) {
+  lldb_private::StringList &lines) {
   // We'll call ourselves complete if we've received a balanced set of braces.
   int start_block_count = 0;
   int brace_balance = 0;
Index: lldb/source/Host/common/Editline.cpp
===
--- lldb/source/Host/common/Editline.cpp
+++ lldb/source/Host/common/Editline.cpp
@@ -641,8 +641,7 @@
   lines.AppendString(new_line_fragment);
 #endif
 
-  int indent_correction = m_fix_indentation_callback(
-  this, lines, 0, m_fix_indentation_callback_baton);
+  int indent_correction = m_fix_indentation_callback(this, lines, 0);
   new_line_fragment = FixIndentation(new_line_fragment, indent_correction);
   m_revert_cursor_index = GetIndentation(new_line_fragment);
 }
@@ -677,8 +676,7 @@
   info->cursor == info->lastchar) {
 if (m_is_input_complete_callback) {
   auto lines = GetInputAsStringList();
-  if (!m_is_input_complete_callback(this, lines,
-m_is_input_complete_callback_baton)) {
+  if (!m_is_input_complete_callback(this, lines)) {
 return BreakLineCommand(ch);
   }
 
@@ -811,8 +809,7 @@
 if (m_fix_indentation_callback) {
   StringList lines = GetInputAsStringList();
   lines.AppendString("");
-  indentation = m_fix_indentation_callback(
-  this, lines, 0, m_fix_indentation_callback_baton);
+  indentation = m_fix_indentation_callback(this, lines, 0);
 }
 m_input_lines.insert(
 m_input_lines.end(),
@@ -857,8 +854,8 @@
   // Save the edits and determine the correct indentation level
   SaveEditedLine();
   StringList lines = GetInputAsStringList(m_current_line_index + 1);
-  int indent_correction = m_fix_indentation_callback(
-  this, lines, cursor_position, m_fix_indentation_callback_baton);
+  int indent_correction =
+  m_fix_indentation_callback(this, lines, cursor_position);
 
   // If it is already correct no special work is needed
   if (indent_correction == 0)
@@ -977,7 +974,7 @@
 }
 
 unsigned char Editline::TabCommand(int ch) {
-  if (m_completion_callback == nullptr)
+  if (!m_completion_callback)
 return CC_ERROR;
 
   const LineInfo *line_info = el_line(m_editline);
@@ -988,7 +985,7 @@
   CompletionResult result;
   CompletionRequest request(line, cursor_index, result);
 
-  m_completion_callback(request, m_completion_callback_baton);
+  m_completion_callback(request);
 
   llvm::ArrayRef results = result.GetResults();
 
@@ -1047,12 +1044,15 @@
 }
 
 unsigned char Editline::ApplyAutosuggestCommand(int ch) {
+  if (!m_suggestion_callback) {
+return CC_REDISPLAY;
+  }
+
   const LineInfo *line_info = el_line(m_editline);
   llvm::StringRef line(line_info->buffer,
line_info->lastchar - line_info->buffer);
 
-  if (llvm::Optional to_add =
-  m_suggestion_callback(line, m_suggestion_callback_baton))
+  if (llvm::Optional to_add = m_suggestion_callback(line))
 el_insertstr(m_editline, to_add->c_str());
 
   return CC_REDISPLAY;
@@ -1061,12 +1061,16 @@
 unsigned char Editline::TypedCharacter(int ch) {
   std::string typed = std::string(1, ch);
   el_insertstr(m_editline, typed.c_str());
+
+  if (!m_suggestion_callback) {
+