[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-05-11 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett added a comment.

  Can you guide me on how I might correct this?

Actually, now I see that MCContext has reportWarning/reportError and just needs 
a reportNote. I'll have something for you to review shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-05-11 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett added a comment.

@ychen Since this change the warning emitted in `AsmPrinterInlineAsm.cpp` about 
reserved registers no longer has location information for the file that the 
inline asm is in. Here's an example:

  void bar(void)
  {
  __asm__ __volatile__ ( "nop" : : : "sp");
  }
  
  ./bin/clang --target=arm-arm-none-eabi -march=armv7-m -c /tmp/test.c -o 
/dev/null

Before:

  /tmp/test.c:3:28: warning: inline asm clobber list contains reserved 
registers: SP [-Winline-asm]
  __asm__ __volatile__ ( "nop" : : : "sp");
 ^
  :1:1: note: instantiated into assembly here
  nop
  ^
  /tmp/test.c:3:28: note: Reserved registers on the clobber list may not be 
preserved across the asm statement, and clobbering them may lead to undefined 
behaviour.
  __asm__ __volatile__ ( "nop" : : : "sp");
 ^
  :1:1: note: instantiated into assembly here
  nop
  ^
  1 warning generated.

After:

  :1:1: warning: inline asm clobber list contains reserved 
registers: SP
  nop
  
  :1:1: note: Reserved registers on the clobber list may not be 
preserved across the asm statement, and clobbering them may lead to undefined 
behaviour.
  nop
  

(the reason it didn't break any tests is that we only check this message from 
IR and only look for the "contains reserved registers" line)

Can you guide me on how I might correct this?

My current impression is that since this code does `SrcMgr.PrintMessage(Loc, 
SourceMgr::DK_Warning, Msg)` it's one level too deep to get source information 
added to it automatically. And that if this code could use `DK_InlineAsm` that 
would help. However getting hold of the Diagnostic manager from MCContext isn't 
possible currently, perhaps that is by design?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Yuanfang Chen via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5de2d189e6ad: [Diagnose] Unify MCContext and LLVMContext 
diagnosing (authored by ychen).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

Files:
  clang/include/clang/Basic/DiagnosticCategories.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  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/DiagnosticInfo.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,7 +1,7 @@
-; 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 -filetype=null < %s 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga -filetype=null < %s 2>&1 | FileCheck %s
 
-; CHECK: lds: unsupported initializer for address space
+; CHECK: error: lds: unsupported initializer for address space
 
 @lds = addrspace(3) global [256 x i32] zeroinitializer
 
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::DiagHand

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Yuanfang Chen via Phabricator via cfe-commits 
ychen updated this revision to Diff 327304.
ychen added a comment.

- Remove dead code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

Files:
  clang/include/clang/Basic/DiagnosticCategories.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  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/DiagnosticInfo.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,7 +1,7 @@
-; 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 -filetype=null < %s 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga -filetype=null < %s 2>&1 | FileCheck %s
 
-; CHECK: lds: unsupported initializer for address space
+; CHECK: error: lds: unsupported initializer for address space
 
 @lds = addrspace(3) global [256 x i32] zeroinitializer
 
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_ostrea

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay accepted this revision.
MaskRay added inline comments.



Comment at: llvm/include/llvm/MC/MCContext.h:67
   class SourceMgr;
+  template  class function_ref;
 

With `std::function`, this can be dropped.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Yuanfang Chen via Phabricator via cfe-commits 
ychen added a comment.

In D97449#2595415 , @MaskRay wrote:

> LG. Can you attach an example triggering clang `InlineAsmDiagHandler2`? I 
> want to check what the diagnostics look like before and now.

`clang/test/CodeGen/asm-errors.c` should trigger it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Yuanfang Chen via Phabricator via cfe-commits 
ychen updated this revision to Diff 327300.
ychen added a comment.

- Revert to use std::function since MCContext needs to own the handler callback
- Fix a typo in BackendConsumer::SrcMgrDiagHandler: DI.getKind() -> 
DI.getSeverity()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

Files:
  clang/include/clang/Basic/DiagnosticCategories.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  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/DiagnosticInfo.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,7 +1,7 @@
-; 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 -filetype=null < %s 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga -filetype=null < %s 2>&1 | FileCheck %s
 
-; CHECK: lds: unsupported initializer for address space
+; CHECK: error: lds: unsupported initializer for address space
 
 @lds = addrspace(3) global [256 x i32] zeroinitializer
 
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(

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Yuanfang Chen via Phabricator via cfe-commits 
ychen added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticFrontendKinds.td:22
 def note_fe_inline_asm_here : Note<"instantiated into assembly here">;
+def err_fe_source_mgr : Error<"%0">, CatSourceMgr;
+def warn_fe_source_mgr : Warning<"%0">, CatSourceMgr, 
InGroup;

MaskRay wrote:
> These seem unused.
These `.td` file changes are used by `ComputeDiagID(DI.getKind(), source_mgr, 
DiagID);` down below. Basically it makes non-inlineasm SourceMgr error its own 
diagnose category.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Yuanfang Chen via Phabricator via cfe-commits 
ychen added inline comments.



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

MaskRay wrote:
> ychen wrote:
> > MaskRay wrote:
> > > Use lightweight function_ref since you don't need to store the callback.
> > I was hesitant to do this because it requires including `STLExtras.h` in 
> > MCContext.h which could be bad for compile-time. 
> You can forward declare `function_ref` (see 
> `mlir/include/mlir/Support/LLVM.h`).
Thanks. Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Yuanfang Chen via Phabricator via cfe-commits 
ychen updated this revision to Diff 327256.
ychen added a comment.

- Use function_ref


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

Files:
  clang/include/clang/Basic/DiagnosticCategories.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  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/DiagnosticInfo.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,7 +1,7 @@
-; 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 -filetype=null < %s 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga -filetype=null < %s 2>&1 | FileCheck %s
 
-; CHECK: lds: unsupported initializer for address space
+; CHECK: error: lds: unsupported initializer for address space
 
 @lds = addrspace(3) global [256 x i32] zeroinitializer
 
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_ostrea

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

LG. Can you attach an example triggering clang `InlineAsmDiagHandler2`? I want 
to check what the diagnostic looks like now.




Comment at: clang/include/clang/Basic/DiagnosticFrontendKinds.td:22
 def note_fe_inline_asm_here : Note<"instantiated into assembly here">;
+def err_fe_source_mgr : Error<"%0">, CatSourceMgr;
+def warn_fe_source_mgr : Warning<"%0">, CatSourceMgr, 
InGroup;

These seem unused.



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1148
 def BackendInlineAsm : DiagGroup<"inline-asm">;
+def BackendSourceMgr : DiagGroup<"source-mgr">;
 def BackendFrameLargerThanEQ : DiagGroup<"frame-larger-than=">;

Unused


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Yuanfang Chen via Phabricator via cfe-commits 
ychen added a comment.

In D97449#2595390 , @MaskRay wrote:

> Is the https://github.com/ClangBuiltLinux/linux/issues/1269 unreachable error 
> fixed?
>
> Reproduce:
>
>   git clone https://android.googlesource.com/kernel/common.git --branch 
> android-4.19-stable
>   cd common
>   # ninja -C path/yo/your/build clang lld check-llvm-tools
>   PATH=path/to/your/build/bin:$PATH make -s -j 30 LLVM=1 O=/tmp/out/android 
> gki_defconfig bzImage 
>   # very slow

I think so. `DiagnosticInfoSrcMgr::print` is now implemented.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

Is the https://github.com/ClangBuiltLinux/linux/issues/1269 unreachable error 
fixed?

Reproduce:

  git clone https://android.googlesource.com/kernel/common.git --branch 
android-4.19-stable
  cd common
  PATH=path/to/your/clang/bin:$PATH make -s -j 30 LLVM=1 O=/tmp/out/android 
gki_defconfig bzImage 
  # very slow




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

ychen wrote:
> MaskRay wrote:
> > Use lightweight function_ref since you don't need to store the callback.
> I was hesitant to do this because it requires including `STLExtras.h` in 
> MCContext.h which could be bad for compile-time. 
You can forward declare `function_ref` (see `mlir/include/mlir/Support/LLVM.h`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Yuanfang Chen via Phabricator via cfe-commits 
ychen updated this revision to Diff 327162.
ychen added a comment.

- Use StringRef::consume_front


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

Files:
  clang/include/clang/Basic/DiagnosticCategories.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  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/DiagnosticInfo.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,7 +1,7 @@
-; 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 -filetype=null < %s 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga -filetype=null < %s 2>&1 | FileCheck %s
 
-; CHECK: lds: unsupported initializer for address space
+; CHECK: error: lds: unsupported initializer for address space
 
 @lds = addrspace(3) global [256 x i32] zeroinitializer
 
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);

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Yuanfang Chen via Phabricator via cfe-commits 
ychen added inline comments.



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

MaskRay wrote:
> This looks a bit strange: we need to construct a fresh SourceMgr to print a 
> diagnostic.
I've included comments to explain that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Yuanfang Chen via Phabricator via cfe-commits 
ychen added inline comments.



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

MaskRay wrote:
> `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?)
It was introduced by 
https://github.com/llvm/llvm-project/commit/5ec32e7fd845e0b7db33689f33cc2ef7c83710fa.
 

I guess it is to canonicalize error messages in case the user just throws in an 
"error: " prefix like `Ctx.diagnose("error: xxx")` which would give two `error` 
prefixes otherwise.



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

MaskRay wrote:
> Use lightweight function_ref since you don't need to store the callback.
I was hesitant to do this because it requires including `STLExtras.h` in 
MCContext.h which could be bad for compile-time. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-03-01 Thread Yuanfang Chen via Phabricator via cfe-commits 
ychen updated this revision to Diff 327155.
ychen added a comment.

- Add clang Diagnostic Category/Group/Kinds for SourceMgr errors
- Add comments in MCContext
- Simplify Clang handler
- Implement DiagnosticInfoSrcMgr::print
- Simplify DiagnosticInfoSrcMgr ctor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

Files:
  clang/include/clang/Basic/DiagnosticCategories.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  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/DiagnosticInfo.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,7 +1,7 @@
-; 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 -filetype=null < %s 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga -filetype=null < %s 2>&1 | FileCheck %s
 
-; CHECK: lds: unsupported initializer for address space
+; CHECK: error: lds: unsupported initializer for address space
 
 @lds = addrspace(3) global [256 x i32] zeroinitializer
 
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.

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-26 Thread Fangrui Song via Phabricator via cfe-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

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-26 Thread Fangrui Song via Phabricator via cfe-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

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-26 Thread Nick Desaulniers via Phabricator via cfe-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

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-26 Thread Yuanfang Chen via Phabricator via cfe-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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-25 Thread Yuanfang Chen via Phabricator via cfe-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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-25 Thread Yuanfang Chen via Phabricator via cfe-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

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-25 Thread Yuanfang Chen via Phabricator via cfe-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

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-25 Thread Reid Kleckner via Phabricator via cfe-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

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

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-02-24 Thread Yuanfang Chen via Phabricator via cfe-commits 
ychen created this revision.
ychen added reviewers: MaskRay, rnk, tejohnson, qcolombet, anemet.
Herald added subscribers: dexonsmith, kerbowa, hiraditya, nhaehnle, jvesely.
ychen requested review of this revision.
Herald added projects: clang, LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits.

The situation with inline asm/MC error reporting is kind of messy at the
moment. The errors from MC layout are not reliably propagated(D96931 
) and users
have to specify an inlineasm handler separately to get inlineasm
diagnose. The latter issue is not a correctness issue but could be improved.

- Kill LLVMContext inlineasm diagnose handler and migrate it to use

DiagnoseInfo/DiagnoseHandler.

- Introduce `DiagnoseInfoSrcMgr` to diagnose SourceMgr backed errors.

This covers use cases like inline asm, MC and any clients using
SourceMgr.

- Move AsmPrinter::SrcMgrDiagInfo and its instance to MCContext. The

next step is to combine MCContext::SrcMgr and MCContext::InlineSrcMgr
because in all use cases, only one of them is used.

- If LLVMContext is available, let MCContext uses LLVMContext's diagnose

handler; if LLVMContext is not available, MCContext uses its own default
diagnose handler which just print SMDiagnostic.

- Change a few clients(Clang, llc, lldb) to use new way of reporting.


Repository:
  rG LLVM Github Monorepo

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
@@