[PATCH] D108499: [clang][codegen] Don't assert on CurLinkModule for silenced diagnostic

2021-08-20 Thread Rong Xu via Phabricator via cfe-commits
xur added a comment.

This looks fine to me. But I agree you that a more complete fix would be having 
CurLinkModule set for all callers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108499

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


[PATCH] D108499: [clang][codegen] Don't assert on CurLinkModule for silenced diagnostic

2021-08-20 Thread Bob Haarman via Phabricator via cfe-commits
inglorion added a comment.

There are some possible follow-ups here:

1. Making sure CurLinkModule is set in all (or at least more) cases.
2. Addressing the FIXME about warnings and notes.

I have some partial implementations of those ideas, but I figure we can take 
this change first while I figure out the other bits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108499

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


[PATCH] D108499: [clang][codegen] Don't assert on CurLinkModule for silenced diagnostic

2021-08-20 Thread Bob Haarman via Phabricator via cfe-commits
inglorion created this revision.
inglorion requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes PR51564.

Under certain circumstances, it is possible to enter Clang's
BackendConsumer::DiagnosticHandlerImpl without CurLinkModule set.
For diagnostics of kind DK_Linker, this results in an assert.
However, these diagnostics are never actually surfaced, unless
they are errors. Crashing the compiler for diagnostics we don't
actually surface seems not worth it, so this change moves the
assert below the check so that it only triggers for diagnostics
we actually surface.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108499

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/CodeGen/Inputs/linker-diagnostic1.ll
  clang/test/CodeGen/linker-diagnostic.ll


Index: clang/test/CodeGen/linker-diagnostic.ll
===
--- /dev/null
+++ clang/test/CodeGen/linker-diagnostic.ll
@@ -0,0 +1,15 @@
+; RUN: mkdir -p %t
+; RUN: opt -module-summary -o %t/foo.o %s
+; RUN: opt -module-summary -o %t/bar.o %S/Inputs/linker-diagnostic1.ll
+; RUN: llvm-lto2 run --thinlto-distributed-indexes -r %t/foo.o,foo,plx -r 
%t/bar.o,bar,plx \
+; RUN:   -r %t/bar.o,foo, -o %t/foobar.so %t/foo.o %t/bar.o
+; RUN: %clang -c -o %t/lto.bar.o --target=armv6-none-unknown-eabi -O2 \
+; RUN:   -fthinlto-index=%t/bar.o.thinlto.bc %t/bar.o
+
+target triple = "thumbv6-unknown-linux-gnueabihf"
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+
+define i32 @foo(i32 %x) {
+  %1 = add i32 %x, 1
+  ret i32 %1
+}
Index: clang/test/CodeGen/Inputs/linker-diagnostic1.ll
===
--- /dev/null
+++ clang/test/CodeGen/Inputs/linker-diagnostic1.ll
@@ -0,0 +1,9 @@
+target triple = "armv4-none-unknown-eabi"
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+
+declare i32 @foo(i32)
+
+define i32 @bar(i32 %x) {
+  %1 = tail call i32 @foo(i32 %x)
+  ret i32 %1
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -779,10 +779,10 @@
 ComputeDiagID(Severity, backend_frame_larger_than, DiagID);
 break;
   case DK_Linker:
-assert(CurLinkModule);
 // FIXME: stop eating the warnings and notes.
 if (Severity != DS_Error)
   return;
+assert(CurLinkModule);
 DiagID = diag::err_fe_cannot_link_module;
 break;
   case llvm::DK_OptimizationRemark:


Index: clang/test/CodeGen/linker-diagnostic.ll
===
--- /dev/null
+++ clang/test/CodeGen/linker-diagnostic.ll
@@ -0,0 +1,15 @@
+; RUN: mkdir -p %t
+; RUN: opt -module-summary -o %t/foo.o %s
+; RUN: opt -module-summary -o %t/bar.o %S/Inputs/linker-diagnostic1.ll
+; RUN: llvm-lto2 run --thinlto-distributed-indexes -r %t/foo.o,foo,plx -r %t/bar.o,bar,plx \
+; RUN:   -r %t/bar.o,foo, -o %t/foobar.so %t/foo.o %t/bar.o
+; RUN: %clang -c -o %t/lto.bar.o --target=armv6-none-unknown-eabi -O2 \
+; RUN:   -fthinlto-index=%t/bar.o.thinlto.bc %t/bar.o
+
+target triple = "thumbv6-unknown-linux-gnueabihf"
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+
+define i32 @foo(i32 %x) {
+  %1 = add i32 %x, 1
+  ret i32 %1
+}
Index: clang/test/CodeGen/Inputs/linker-diagnostic1.ll
===
--- /dev/null
+++ clang/test/CodeGen/Inputs/linker-diagnostic1.ll
@@ -0,0 +1,9 @@
+target triple = "armv4-none-unknown-eabi"
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+
+declare i32 @foo(i32)
+
+define i32 @bar(i32 %x) {
+  %1 = tail call i32 @foo(i32 %x)
+  ret i32 %1
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -779,10 +779,10 @@
 ComputeDiagID(Severity, backend_frame_larger_than, DiagID);
 break;
   case DK_Linker:
-assert(CurLinkModule);
 // FIXME: stop eating the warnings and notes.
 if (Severity != DS_Error)
   return;
+assert(CurLinkModule);
 DiagID = diag::err_fe_cannot_link_module;
 break;
   case llvm::DK_OptimizationRemark:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits