llvmorg-github-actions[bot] wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-x86

Author: pgerell

<details>
<summary>Changes</summary>

Fixes #<!-- -->147515.

## The bug

```c++
// t.cpp
void foo();

int f() {
  try {
    foo();
  } catch (...) {
    return 1;
  }
  return 0;
}
```

```
$ clang++ -fsyntax-only -fno-exceptions t.cpp
t.cpp:4:3: error: cannot use 'try' with exceptions disabled

$ clang++ -fsyntax-only -fno-exceptions -fopenmp t.cpp
$ clang++ -fsyntax-only -fno-exceptions -fopenmp-simd t.cpp
$ clang++ -fsyntax-only -fno-exceptions -fopenmp 
-fopenmp-targets=x86_64-unknown-linux-gnu t.cpp
t.cpp:4:3: error: cannot use 'try' with exceptions disabled
```

Adding `-fopenmp` drops the error. Adding an offload target brings it back.

## Why

`Sema::targetDiag` sends the diagnostic to `SemaOpenMP::diagIfOpenMPHostCode`, 
which
queues it as `K_Deferred`. Nothing flushes that queue, because
`Sema::ActOnFinishFunctionBody` only records a function for deferred-diagnostic 
checking
when

```c++
LangOpts.isTargetDevice() || LangOpts.CUDA ||
    (LangOpts.OpenMP &amp;&amp; !LangOpts.OMPTargetTriples.empty())
```

With plain `-fopenmp` there are no target triples, so nothing is recorded and
`Sema::emitDeferredDiags` returns early on 
`DeclsToCheckForDeferredDiags.empty()`. The
deferral itself is fine, the flush is missing.

Every diagnostic routed through `targetDiag` is affected, not just this one. 
For inline
asm it is worse than a missing error, because `SemaStmtAsm.cpp` continues with
`return CreateGCCAsmStmt()` afterwards, so an invalid constraint reaches 
CodeGen:

```c++
void g(int x) { __asm__("nop" : "=@"(x)); }
```

```
$ clang++ -fopenmp -c a.cpp
clang/lib/CodeGen/CGStmt.cpp:2912: Assertion `IsValid &amp;&amp; "Failed to 
parse output constraint"' failed.
```

`"Failed to parse input constraint"` in the same file and
`isValidGCCRegisterName(Name) &amp;&amp; "Invalid register passed in"` in 
`TargetInfo.cpp` are
reachable the same way.

## The fix

Two changes. Neither is sufficient alone.

`ActOnFinishFunctionBody` now records functions on any OpenMP compilation.
`getEmissionStatus(FD, /*Final=*/true)` still reports `OMPDiscarded` for
`device_type(nohost)`, so functions the host does not emit keep their 
diagnostics
suppressed, including when the `declare target` comes after the body.

`Sema::targetDiag` no longer routes `-fopenmp-simd` into the deferral path. 
There is no
device compilation to defer for, and `declare target` is ignored, so a `nohost` 
function
is in fact emitted on the host under `-fopenmp-simd` while its diagnostics were 
being
suppressed as if it were not.

Both are needed because `-fopenmp` does not emit functions eagerly
(`CodeGenModule::MayBeEmittedEagerly`), so the end-of-TU flush still runs 
before CodeGen.
Under `-fopenmp-simd` the function is emitted while parsing and CodeGen asserts 
first, so
the diagnostic has to be immediate.

## Tests

Three existing tests had the old behaviour written into them. The `-fopenmp` 
RUN line of
`CodeGen/X86/mmx-inline-asm-error.c` expected different diagnostics from its 
default run,
and `OpenMP/declare_target_messages.cpp` expected the `device_type(nohost)` 
diagnostics
only on the RUN lines passing `-fopenmp-targets`. `OpenMP/target_ast_print.cpp` 
would
also fail with these diagnostics. #<!-- -->220058 fixed it, and that PR is now 
in main.

New: `OpenMP/host_exceptions_messages.cpp`, `OpenMP/host_asm_messages.c`.

The contents of this PR is created with help from Claude Code and ChatGPT.


---
Full diff: https://github.com/llvm/llvm-project/pull/220316.diff


7 Files Affected:

- (modified) clang/docs/ReleaseNotes.md (+11) 
- (modified) clang/lib/Sema/Sema.cpp (+3-1) 
- (modified) clang/lib/Sema/SemaDecl.cpp (+1-2) 
- (modified) clang/test/CodeGen/X86/mmx-inline-asm-error.c (+2-5) 
- (modified) clang/test/OpenMP/declare_target_messages.cpp (+6-6) 
- (added) clang/test/OpenMP/host_asm_messages.c (+9) 
- (added) clang/test/OpenMP/host_exceptions_messages.cpp (+37) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md
index 7ec126a065ae5..5dbc0eaf85421 100644
--- a/clang/docs/ReleaseNotes.md
+++ b/clang/docs/ReleaseNotes.md
@@ -129,6 +129,10 @@ features cannot lower the translation-unit ABI level;
   target triples (except 32b arm targets). Can be disabled via
   `-fno-stack-clash-protection`.
 
+- A host `-fopenmp` or `-fopenmp-simd` compilation now emits diagnostics it 
previously
+  deferred and then dropped when no offload target was configured. Code that 
used to
+  compile may be rejected, matching a compilation without those flags.
+
 ### Clang Python Bindings Potentially Breaking Changes
 
 - `CompletionChunkKind` instance's `__str__` representation has been adapted 
to be consistent with other enums in the library.
@@ -1127,6 +1131,13 @@ The `alpha.cplusplus.UseAfterLifetimeEnd` checker was 
renamed to `alpha.core.Use
   To ensure correct behavior, either use the original variable directly in the
   target region or map the bindings explicitly instead.
 
+- Fixed a host OpenMP compilation never flushing its deferred diagnostics 
unless an
+  offload target was configured, which accepted `try`/`throw` with exceptions 
disabled
+  and let an invalid inline asm constraint crash CodeGen. (#GH147515)
+- Fixed `-fopenmp-simd` deferring diagnostics as if it were a device 
compilation. It has
+  no device compilation and ignores `declare target`, so its diagnostics are 
now emitted
+  directly.
+
 ### SYCL Support
 
 #### Improvements
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 29158cfff6231..c9b894eac406b 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -2262,7 +2262,9 @@ Sema::SemaDiagnosticBuilder::~SemaDiagnosticBuilder() {
 Sema::SemaDiagnosticBuilder
 Sema::targetDiag(SourceLocation Loc, unsigned DiagID, const FunctionDecl *FD) {
   FD = FD ? FD : getCurFunctionDecl();
-  if (LangOpts.OpenMP)
+  // -fopenmp-simd has no device compilation and ignores 'declare target', so
+  // there is nothing to defer for.
+  if (LangOpts.OpenMP && !LangOpts.OpenMPSimd)
     return LangOpts.OpenMPIsTargetDevice
                ? OpenMP().diagIfOpenMPDeviceCode(Loc, DiagID, FD)
                : OpenMP().diagIfOpenMPHostCode(Loc, DiagID, FD);
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 094601a58d508..0e4e133094f72 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -17375,8 +17375,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt 
*Body, bool IsInstantiation,
     DiscardCleanupsInEvaluationContext();
   }
 
-  if (FD && (LangOpts.isTargetDevice() || LangOpts.CUDA ||
-             (LangOpts.OpenMP && !LangOpts.OMPTargetTriples.empty()))) {
+  if (FD && (LangOpts.isTargetDevice() || LangOpts.CUDA || LangOpts.OpenMP)) {
     auto ES = getEmissionStatus(FD);
     if (ES == Sema::FunctionEmissionStatus::Emitted ||
         ES == Sema::FunctionEmissionStatus::Unknown)
diff --git a/clang/test/CodeGen/X86/mmx-inline-asm-error.c 
b/clang/test/CodeGen/X86/mmx-inline-asm-error.c
index 7f7f53a553057..da5b60fdac64c 100644
--- a/clang/test/CodeGen/X86/mmx-inline-asm-error.c
+++ b/clang/test/CodeGen/X86/mmx-inline-asm-error.c
@@ -1,16 +1,13 @@
 // RUN: %clang_cc1 -verify -triple x86_64-unknown-unknown -emit-llvm-only %s
-// RUN: %clang_cc1 -verify=omp -triple x86_64-unknown-unknown -emit-llvm-only 
-fopenmp %s
+// RUN: %clang_cc1 -verify -triple x86_64-unknown-unknown -emit-llvm-only 
-fopenmp %s
+// RUN: %clang_cc1 -verify -triple x86_64-unknown-unknown -emit-llvm-only 
-fopenmp-simd %s
 typedef int vec256 __attribute__((ext_vector_type(8)));
 
-// omp-warning@+2 {{AVX vector return of type 'vec256' (vector of 8 'int' 
values) without 'avx' enabled changes the ABI}}
-// omp-warning@+1 {{AVX vector argument of type 'vec256' (vector of 8 'int' 
values) without 'avx' enabled changes the ABI}}
 vec256 foo(vec256 in) {
   vec256 out;
 
   asm("something %0" : : "y"(in)); // expected-error {{invalid input size for 
constraint 'y'}}
-  // omp-error@+1 {{invalid type 'vec256' (vector of 8 'int' values) in asm 
input for constraint 'y'}}
   asm("something %0" : "=y"(out)); // expected-error {{invalid output size for 
constraint '=y'}}
-  // omp-error@+1 {{invalid type 'vec256' (vector of 8 'int' values) in asm 
input for constraint 'y'}}
   asm("something %0, %0" : "+y"(out)); // expected-error {{invalid output size 
for constraint '+y'}}
 
   return out;
diff --git a/clang/test/OpenMP/declare_target_messages.cpp 
b/clang/test/OpenMP/declare_target_messages.cpp
index 6fe477755dbe7..62a12d40b7271 100644
--- a/clang/test/OpenMP/declare_target_messages.cpp
+++ b/clang/test/OpenMP/declare_target_messages.cpp
@@ -21,16 +21,16 @@
 // RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp60,omp52-or-later,ompvar,omp5-or-later,omp5-or-later-var 
%{openmp60_simd} -fopenmp-is-target-device %{target_mac} %{limit} -o - %s
 
 // RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp45,omp45-to-51,omp45-to-51-var,omp45-to-51-clause 
-fopenmp-version=45 -fopenmp-simd %{limit} -o - %s
-// RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp51,ompvar,omp45-to-51,omp5-and-51,omp5-or-later,omp5-or-later-var,omp45-to-51-var,omp45-to-51-clause,host-5-and-51,no-host5-and-51
 -fopenmp %{limit} -o - %s
-// RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp51,ompvar,omp45-to-51,omp5-and-51,omp5-or-later,omp5-or-later-var,omp45-to-51-var,omp45-to-51-clause,host-5-and-51,no-host5-and-51
 -fopenmp %{limit} -DTESTEND=1 -o - %s
-// RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp51,ompvar,omp45-to-51,omp5-and-51,omp5-or-later,omp5-or-later-var,omp45-to-51-var,omp45-to-51-clause,host-5-and-51,no-host5-and-51
 -fopenmp %{limit} -I%S/Inputs -DTESTENDINC=1 -o - %s
-// RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp51,ompvar,omp45-to-51,omp5-and-51,omp5-or-later,omp5-or-later-var,omp45-to-51-var,omp45-to-51-clause,host-5-and-51,no-host5-and-51
 -fopenmp-simd %{limit} -o - %s
+// RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp51,ompvar,omp45-to-51,omp5-and-51,omp5-or-later,omp5-or-later-var,omp45-to-51-var,omp45-to-51-clause,host5,host-5-and-51,no-host5-and-51
 -fopenmp %{limit} -o - %s
+// RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp51,ompvar,omp45-to-51,omp5-and-51,omp5-or-later,omp5-or-later-var,omp45-to-51-var,omp45-to-51-clause,host5,host-5-and-51,no-host5-and-51
 -fopenmp %{limit} -DTESTEND=1 -o - %s
+// RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp51,ompvar,omp45-to-51,omp5-and-51,omp5-or-later,omp5-or-later-var,omp45-to-51-var,omp45-to-51-clause,host5,host-5-and-51,no-host5-and-51
 -fopenmp %{limit} -I%S/Inputs -DTESTENDINC=1 -o - %s
+// RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp51,ompvar,omp45-to-51,omp5-and-51,omp5-or-later,omp5-or-later-var,omp45-to-51-var,omp45-to-51-clause,host5,host-5-and-51,no-host5-and-51
 -fopenmp-simd %{limit} -o - %s
 
 // RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp52,omp52-or-later,ompvar,omp5-or-later,omp5-or-later-var 
%{openmp52} -DVERBOSE_MODE=1 %{limit} -o - %s
 // RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp60,omp52-or-later,ompvar,omp5-or-later,omp5-or-later-var 
%{openmp60} -DVERBOSE_MODE=1 %{limit} -o - %s
 
-// RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp5,ompvar,omp45-to-51,omp5-and-51,omp5-or-later,omp5-or-later-var,omp45-to-51-var,omp45-to-51-clause,host-5-and-51,no-host5-and-51
 %{openmp50} %{limit} -o - %s
-// RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp51,ompvar,omp45-to-51,omp5-and-51,omp5-or-later,omp5-or-later-var,omp45-to-51-var,omp45-to-51-clause,host-5-and-51,no-host5-and-51
 -fopenmp %{limit} -o - %s
+// RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp5,ompvar,omp45-to-51,omp5-and-51,omp5-or-later,omp5-or-later-var,omp45-to-51-var,omp45-to-51-clause,host5,host-5-and-51,no-host5-and-51
 %{openmp50} %{limit} -o - %s
+// RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp51,ompvar,omp45-to-51,omp5-and-51,omp5-or-later,omp5-or-later-var,omp45-to-51-var,omp45-to-51-clause,host5,host-5-and-51,no-host5-and-51
 -fopenmp %{limit} -o - %s
 // RUN: %clang_cc1 %{common_opts_mac} 
-verify=expected,omp60,omp52-or-later,ompvar,omp5-or-later,omp5-or-later-var 
%{openmp60} %{limit} -o - %s
 
 #pragma omp begin declare target
diff --git a/clang/test/OpenMP/host_asm_messages.c 
b/clang/test/OpenMP/host_asm_messages.c
new file mode 100644
index 0000000000000..48126712f5437
--- /dev/null
+++ b/clang/test/OpenMP/host_asm_messages.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -verify -triple x86_64-unknown-unknown -emit-llvm-only %s
+// RUN: %clang_cc1 -verify -triple x86_64-unknown-unknown -emit-llvm-only 
-fopenmp %s
+// RUN: %clang_cc1 -verify -triple x86_64-unknown-unknown -emit-llvm-only 
-fopenmp-simd %s
+
+// An invalid asm constraint must be diagnosed rather than reaching CodeGen.
+
+void f(void) {
+  __asm__("nop" ::: "no_such_register"); // expected-error {{unknown register 
name 'no_such_register' in asm}}
+}
diff --git a/clang/test/OpenMP/host_exceptions_messages.cpp 
b/clang/test/OpenMP/host_exceptions_messages.cpp
new file mode 100644
index 0000000000000..7a176f4df5f8a
--- /dev/null
+++ b/clang/test/OpenMP/host_exceptions_messages.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -verify=expected,host -fopenmp -fsyntax-only %s
+// RUN: %clang_cc1 -verify=expected,simd -fopenmp-simd -fsyntax-only %s
+// RUN: %clang_cc1 -verify=expected,host -fopenmp -fopenmp-targets=x86_64 
-triple x86_64 -fsyntax-only %s
+
+// Exceptions are disabled, so a host compilation must diagnose 'try' and 
'throw'
+// whether or not an offload target is configured.
+
+void foo();
+
+void bar() {
+  try { // expected-error {{cannot use 'try' with exceptions disabled}}
+    foo();
+  } catch (...) {
+  }
+}
+
+void baz(bool b) {
+  if (b)
+    throw 1; // expected-error {{cannot use 'throw' with exceptions disabled}}
+}
+
+// A 'device_type(nohost)' function is not emitted by a host -fopenmp 
compilation, but
+// -fopenmp-simd ignores 'declare target' and does emit it.
+#pragma omp begin declare target device_type(nohost)
+void devonly() {
+  try { // simd-error {{cannot use 'try' with exceptions disabled}}
+    foo();
+  } catch (...) {
+  }
+}
+#pragma omp end declare target
+
+// Same, but marked 'nohost' only after the body has been parsed.
+void late() {
+  throw 1; // simd-error {{cannot use 'throw' with exceptions disabled}}
+}
+#pragma omp declare target to(late) device_type(nohost)

``````````

</details>


https://github.com/llvm/llvm-project/pull/220316
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to