Author: Akash Manna Date: 2026-09-22T07:16:32-04:00 New Revision: 3578fa36b62dc4355225bd6859c0c098f52c29d4
URL: https://github.com/llvm/llvm-project/commit/3578fa36b62dc4355225bd6859c0c098f52c29d4 DIFF: https://github.com/llvm/llvm-project/commit/3578fa36b62dc4355225bd6859c0c098f52c29d4.diff LOG: [clang][OpenMP] Fix statement expressions in loop bounds being emitted more than once (#224939) 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. A bound of a non-rectangular loop can't be captured at all, since it is re-evaluated at every use and the original condition doubles as the body guard, so a statement expression there is now rejected with an error, as GCC already does. Added: clang/test/OpenMP/for_collapse_stmt_expr_codegen.cpp Modified: clang/docs/ReleaseNotes.md clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaOpenMP.cpp clang/test/OpenMP/for_loop_messages.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 303f972fcaae1..ba944a62a1fff 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. A statement expression in a bound of a non-rectangular loop is now diagnosed. (#GH153987) #### Bug Fixes to Compiler Builtins diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 9074dc7a822c5..7190ba9d92685 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -12642,6 +12642,8 @@ def err_omp_invariant_or_linear_dependency : Error< "expected loop invariant expression or '<invariant1> * %0 + <invariant2>' kind of expression">; def err_omp_wrong_dependency_iterator_type : Error< "expected an integer or a pointer type of the outer loop counter '%0' for non-rectangular nests">; +def err_omp_stmt_expr_in_non_rectangular_loop : Error< + "statement expression is not supported in the loop %select{initializer|condition}0 of a non-rectangular loop nest">; def err_target_unsupported_type : Error<"%0 requires %select{|%2 bit size}1 %3 %select{|return }4type support," " but target '%5' does not support it">; diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index c6c8e71abaf5f..88b642b73895f 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -8282,6 +8282,8 @@ class OpenMPIterationSpaceChecker { SourceRange SR, SourceLocation SL); /// Helper to set loop increment. bool setStep(Expr *NewStep, bool Subtract); + /// Diagnose a statement expression in a bound of a non-rectangular loop. + bool checkNonRectangularBound(const Expr *E, bool IsInitializer) const; }; bool OpenMPIterationSpaceChecker::dependent() const { @@ -8294,6 +8296,32 @@ bool OpenMPIterationSpaceChecker::dependent() const { (Step && Step->isValueDependent()); } +/// Find a statement expression in \p E. +static const StmtExpr *findStmtExpr(const Expr *E) { + SmallVector<const Stmt *, 8> Worklist{E}; + while (!Worklist.empty()) { + const Stmt *S = Worklist.pop_back_val(); + if (!S) + continue; + if (const auto *SE = dyn_cast<StmtExpr>(S)) + return SE; + llvm::append_range(Worklist, S->children()); + } + return nullptr; +} + +bool OpenMPIterationSpaceChecker::checkNonRectangularBound( + const Expr *E, bool IsInitializer) const { + // Such a bound is emitted at every use, its declarations cannot be. + const StmtExpr *SE = findStmtExpr(E); + if (!SE) + return false; + SemaRef.Diag(SE->getBeginLoc(), + diag::err_omp_stmt_expr_in_non_rectangular_loop) + << (IsInitializer ? 0 : 1); + return true; +} + bool OpenMPIterationSpaceChecker::setLCDeclAndLB(ValueDecl *NewLCDecl, Expr *NewLCRefExpr, Expr *NewLB, bool EmitDiags) { @@ -8311,8 +8339,11 @@ bool OpenMPIterationSpaceChecker::setLCDeclAndLB(ValueDecl *NewLCDecl, CE->getNumArgs() > 0 && CE->getArg(0) != nullptr) NewLB = CE->getArg(0)->IgnoreParenImpCasts(); LB = NewLB; - if (EmitDiags) + if (EmitDiags) { InitDependOnLC = doesDependOnLoopCounter(LB, /*IsInitializer=*/true); + if (InitDependOnLC && checkNonRectangularBound(LB, /*IsInitializer=*/true)) + return true; + } return false; } @@ -8331,6 +8362,10 @@ bool OpenMPIterationSpaceChecker::setUB(Expr *NewUB, std::optional<bool> LessOp, ConditionSrcRange = SR; ConditionLoc = SL; CondDependOnLC = doesDependOnLoopCounter(UB, /*IsInitializer=*/false); + // The condition is also emitted as the body guard of a non-rectangular loop. + if ((InitDependOnLC || CondDependOnLC) && + checkNonRectangularBound(UB, /*IsInitializer=*/false)) + return true; return false; } @@ -8853,7 +8888,10 @@ tryBuildCapture(Sema &SemaRef, Expr *Capture, 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) && + !findStmtExpr(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 0000000000000..57246403c04ee --- /dev/null +++ b/clang/test/OpenMP/for_collapse_stmt_expr_codegen.cpp @@ -0,0 +1,105 @@ +// 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 @_Z33collapse_stmt_expr_lb_non_rect_ubv( +// CHECK: %c = alloca i32, +// CHECK-NOT: %c{{[0-9]+}} = alloca +// CHECK: store i32 0, ptr %c, +// CHECK-NOT: store i32 0, ptr %c, +// CHECK: ret void +void collapse_stmt_expr_lb_non_rect_ub() { +#pragma omp for collapse(2) + for (int i = 0; i < 10; i++) + for (int j = ({int c = 0; 0; }); 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++) + ; +} diff --git a/clang/test/OpenMP/for_loop_messages.cpp b/clang/test/OpenMP/for_loop_messages.cpp index 5f6f9c9a3fbc9..d2fbeea106a51 100644 --- a/clang/test/OpenMP/for_loop_messages.cpp +++ b/clang/test/OpenMP/for_loop_messages.cpp @@ -308,6 +308,24 @@ int test_iteration_spaces() { for (kk = ii * 10 + 25; kk < jj - 23; kk += 1) ; +// expected-error@+3 {{statement expression is not supported in the loop initializer of a non-rectangular loop nest}} +#pragma omp for collapse(2) + for (ii = 0; ii < 10; ii += 1) + for (kk = ({int s = ii; s; }); kk < 10; kk += 1) + ; + +// expected-error@+3 {{statement expression is not supported in the loop condition of a non-rectangular loop nest}} +#pragma omp for collapse(2) + for (ii = 0; ii < 10; ii += 1) + for (kk = 0; kk < ({int s = ii; s; }); kk += 1) + ; + +// expected-error@+3 {{statement expression is not supported in the loop condition of a non-rectangular loop nest}} +#pragma omp for collapse(2) + for (ii = 0; ii < 10; ii += 1) + for (kk = ii; kk < ({int s = 10; s; }); kk += 1) + ; + #pragma omp parallel // expected-note@+2 {{defined as firstprivate}} // expected-error@+2 {{loop iteration variable in the associated loop of 'omp for' directive may not be firstprivate, predetermined as private}} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
