llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Akash Manna (akash-manna-sky)
<details>
<summary>Changes</summary>
Fixes #<!-- -->153987
When Sema builds the helper expressions for an OpenMP loop, a bound that
evaluates to a constant is reused directly instead of being captured into a
`.capture_expr.` variable, so the same expression node is spliced into the
init, update, final and precondition helpers. That is fine for `10 + 1`, but a
statement expression like `({int a = 0; 0;})` declares a variable, and emitting
the shared node several times re-declares `a` and trips `Decl already exists in
LocalDeclMap!` in CodeGen. The report only looked like a bytecode-interpreter
issue because the old evaluator happened to refuse `({float a = 0; a;})` and
the new one didn't; the unused-variable form crashes with both.
The capture decision now treats any expression containing a statement
expression as non-constant, so it goes through the existing capture path:
evaluated once in the pre-init statement, with every helper reading the
captured value. Nothing changes for other constant bounds.
---
Full diff: https://github.com/llvm/llvm-project/pull/224939.diff
3 Files Affected:
- (modified) clang/docs/ReleaseNotes.md (+1)
- (modified) clang/lib/Sema/SemaOpenMP.cpp (+18-1)
- (added) clang/test/OpenMP/for_collapse_stmt_expr_codegen.cpp (+92)
``````````diff
diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md
index f4a34a37aff52e..25890b7a4094cf 100644
--- a/clang/docs/ReleaseNotes.md
+++ b/clang/docs/ReleaseNotes.md
@@ -547,6 +547,7 @@ features cannot lower the translation-unit ABI level;
- Fixed a crash when an `asm` label names the register for a global variable
of incomplete type. (#GH219746)
- Fixed an ICE hat occurred when using `__imag int/float` as lvalue in
assignment. (#GH119498)
- Fixed an assertion failure in `-Wsign-compare` when a negated or
complemented vector of unsigned integers was compared against a signed
constant. (#GH203575)
+- Fixed an assertion failure when a constant statement expression that
declares a variable is used as a bound of an OpenMP loop. (#GH153987)
#### Bug Fixes to Compiler Builtins
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 2e4d9f2f82f0b7..73d38b17a59dd7 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -8847,13 +8847,30 @@ bool OpenMPIterationSpaceChecker::checkAndSetInc(Expr
*S) {
return true;
}
+/// Check if the expression \p E contains a statement expression.
+static bool containsStmtExpr(const Expr *E) {
+ SmallVector<const Stmt *, 8> Worklist{E};
+ while (!Worklist.empty()) {
+ const Stmt *S = Worklist.pop_back_val();
+ if (!S)
+ continue;
+ if (isa<StmtExpr>(S))
+ return true;
+ llvm::append_range(Worklist, S->children());
+ }
+ return false;
+}
+
static ExprResult
tryBuildCapture(Sema &SemaRef, Expr *Capture,
llvm::MapVector<const Expr *, DeclRefExpr *> &Captures,
StringRef Name = ".capture_expr.") {
if (SemaRef.CurContext->isDependentContext() || Capture->containsErrors())
return Capture;
- if (Capture->isEvaluatable(SemaRef.Context, Expr::SE_AllowSideEffects))
+ // A statement expression must be captured even if it is constant, since the
+ // declarations inside it can only be emitted once.
+ if (Capture->isEvaluatable(SemaRef.Context, Expr::SE_AllowSideEffects) &&
+ !containsStmtExpr(Capture))
return SemaRef.PerformImplicitConversion(Capture->IgnoreImpCasts(),
Capture->getType(),
AssignmentAction::Converting,
diff --git a/clang/test/OpenMP/for_collapse_stmt_expr_codegen.cpp
b/clang/test/OpenMP/for_collapse_stmt_expr_codegen.cpp
new file mode 100644
index 00000000000000..9b43e055e76960
--- /dev/null
+++ b/clang/test/OpenMP/for_collapse_stmt_expr_codegen.cpp
@@ -0,0 +1,92 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown
-emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -fexperimental-new-constant-interpreter -x
c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+
+// RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -triple x86_64-unknown-unknown
-emit-llvm %s -o - | FileCheck %s --implicit-check-not="{{__kmpc|__tgt}}"
+// RUN: %clang_cc1 -verify -fopenmp-simd
-fexperimental-new-constant-interpreter -x c++ -triple x86_64-unknown-unknown
-emit-llvm %s -o - | FileCheck %s --implicit-check-not="{{__kmpc|__tgt}}"
+// expected-no-diagnostics
+
+// CHECK-LABEL: define {{.*}}void @_Z23collapse_stmt_expr_initv(
+// CHECK: %a = alloca float,
+// CHECK-NOT: %a{{[0-9]+}} = alloca
+// CHECK: store float 0.000000e+00, ptr %a,
+// CHECK-NOT: store float 0.000000e+00, ptr %a,
+// CHECK: ret void
+void collapse_stmt_expr_init() {
+#pragma omp for collapse(2)
+ for (int i = ({float a = 0;a; }); i < 10; i++)
+ for (int j = i; j < 10 + i; j++)
+ ;
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z30collapse_stmt_expr_init_unusedv(
+// CHECK: %a = alloca i32,
+// CHECK-NOT: %a{{[0-9]+}} = alloca
+// CHECK: store i32 0, ptr %a,
+// CHECK-NOT: store i32 0, ptr %a,
+// CHECK: ret void
+void collapse_stmt_expr_init_unused() {
+#pragma omp for collapse(2)
+ for (int i = ({int a = 0; 0; }); i < 10; i++)
+ for (int j = i; j < 10 + i; j++)
+ ;
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z23collapse_stmt_expr_condv(
+// CHECK: %b = alloca i32,
+// CHECK-NOT: %b{{[0-9]+}} = alloca
+// CHECK: store i32 10, ptr %b,
+// CHECK-NOT: store i32 10, ptr %b,
+// CHECK: ret void
+void collapse_stmt_expr_cond() {
+#pragma omp for collapse(2)
+ for (int i = 0; i < ({int b = 10; 10; }); i++)
+ for (int j = i; j < 10 + i; j++)
+ ;
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z23collapse_stmt_expr_stepv(
+// CHECK: %c = alloca i32,
+// CHECK-NOT: %c{{[0-9]+}} = alloca
+// CHECK: store i32 1, ptr %c,
+// CHECK-NOT: store i32 1, ptr %c,
+// CHECK: ret void
+void collapse_stmt_expr_step() {
+#pragma omp for collapse(2)
+ for (int i = 0; i < 10; i += ({int c = 1; 1; }))
+ for (int j = i; j < 10 + i; j++)
+ ;
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z14stmt_expr_initv(
+// CHECK: %a = alloca i32,
+// CHECK-NOT: %a{{[0-9]+}} = alloca
+// CHECK: store i32 0, ptr %a,
+// CHECK-NOT: store i32 0, ptr %a,
+// CHECK: ret void
+void stmt_expr_init() {
+#pragma omp for
+ for (int i = ({int a = 0; 0; }); i < 10; i++)
+ ;
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z19simd_stmt_expr_initv(
+// CHECK: %a = alloca i32,
+// CHECK-NOT: %a{{[0-9]+}} = alloca
+// CHECK: store i32 0, ptr %a,
+// CHECK-NOT: store i32 0, ptr %a,
+// CHECK: ret void
+void simd_stmt_expr_init() {
+#pragma omp simd collapse(2)
+ for (int i = ({int a = 0; 0; }); i < 10; i++)
+ for (int j = i; j < 10 + i; j++)
+ ;
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z17collapse_baselinev(
+// CHECK: ret void
+void collapse_baseline() {
+#pragma omp for collapse(2)
+ for (int i = 0; i < 10; i++)
+ for (int j = i; j < 10 + i; j++)
+ ;
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/224939
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
