https://github.com/loopacino created https://github.com/llvm/llvm-project/pull/237907
Flatten was setting inner loop vars even when the outer loop never ran. Fix: Only run inner assignment inits if every enclosing loop has at least one trip. Always keep `DeclStmt` inits so the variable still exists. Handles the flatten half of #225350 >From 3453b2e1f1468e34f70189af0587dba40a12c529 Mon Sep 17 00:00:00 2001 From: amtiwari <[email protected]> Date: Mon, 2 Nov 2026 05:42:33 -0500 Subject: [PATCH] gate_inner_inits --- clang/lib/Sema/SemaOpenMP.cpp | 40 ++++++++++++++++--- clang/test/OpenMP/flatten_codegen.cpp | 12 ++++++ .../test/transform/flatten/final-values.c | 37 +++++++++++++++++ 3 files changed, 84 insertions(+), 5 deletions(-) diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index a3be416d2da24f..8a2134d355faf9 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -17358,12 +17358,24 @@ SemaOpenMP::ActOnOpenMPFlattenDirective(ArrayRef<OMPClause *> Clauses, // Collect pre-init statements in outer-to-inner order. SmallVector<Stmt *> PreInits; + SmallVector<SmallVector<Stmt *, 2>, 4> GatedInits(NumLoops); for (auto I : llvm::seq<unsigned>(NumLoops)) { OMPLoopBasedDirective::HelperExprs &LoopHelper = LoopHelpers[I]; assert(LoopHelper.Counters.size() == 1 && "Single-dimensional loop iteration space expected"); - addLoopPreInits(Context, LoopHelper, LoopStmts[I], OriginalInits[I], - PreInits); + if (I == 0) { + addLoopPreInits(Context, LoopHelper, LoopStmts[I], OriginalInits[I], + PreInits); + continue; + } + SmallVector<Stmt *, 2> AlwaysInits; + for (Stmt *S : OriginalInits[I]) { + if (isa<DeclStmt>(S)) + AlwaysInits.push_back(S); + else + GatedInits[I].push_back(S); + } + addLoopPreInits(Context, LoopHelper, LoopStmts[I], AlwaysInits, PreInits); } CaptureVars CopyTransformer(SemaRef); @@ -17463,6 +17475,23 @@ SemaOpenMP::ActOnOpenMPFlattenDirective(ArrayRef<OMPClause *> Clauses, if (!TripCount.isUsable()) return StmtError(); + // Inner assignments run only if every enclosing loop has at least one + // iteration. + for (unsigned I = 1; I < NumLoops; ++I) { + if (GatedInits[I].empty()) + continue; + ExprResult InitCond = BuildHasIterations(/*Count=*/I); + if (!InitCond.isUsable()) + return StmtError(); + Stmt *ThenStmt = GatedInits[I].front(); + if (GatedInits[I].size() > 1) + ThenStmt = CompoundStmt::Create(Context, GatedInits[I], + FPOptionsOverride(), CondLoc, CondLoc); + PreInits.push_back(IfStmt::Create( + Context, CondLoc, IfStatementKind::Ordinary, nullptr, nullptr, + InitCond.get(), CondLoc, CondLoc, ThenStmt, SourceLocation(), nullptr)); + } + auto MakeNumIterationsInIVTy = [&](unsigned I) -> Expr * { return AssertSuccess(SemaRef.PerformImplicitConversion( MakeNumIterations(I), IVTy, AssignmentAction::Converting, @@ -17588,9 +17617,10 @@ SemaOpenMP::ActOnOpenMPFlattenDirective(ArrayRef<OMPClause *> Clauses, OutermostHelper.Inc->getEndLoc()); // A counter only reaches its final value if its own loop and all enclosing - // loops execute at least one iteration; otherwise it keeps the value assigned - // by the pre-inits. Guarding also avoids evaluating 'start + n * step' for an - // empty loop, whose trip count may have wrapped. + // loops execute at least one iteration. Otherwise it keeps the last value + // from its init, or the value from before the nest if that init did not run. + // Guarding also avoids evaluating 'start + n * step' for an empty loop, + // whose trip count may have wrapped. SmallVector<Stmt *, 4> Finals; for (unsigned I = 0; I < NumLoops; ++I) { assert(LoopHelpers[I].Finals.size() == 1 && diff --git a/clang/test/OpenMP/flatten_codegen.cpp b/clang/test/OpenMP/flatten_codegen.cpp index 109aa16e3e0790..d1aec7e1c224f3 100644 --- a/clang/test/OpenMP/flatten_codegen.cpp +++ b/clang/test/OpenMP/flatten_codegen.cpp @@ -114,4 +114,16 @@ extern "C" void mixed_sign(int n) { body((int)i, j); } +// Empty outer loop: the outer init still runs, the inner init must not. +// CHECK-LABEL: define {{.*}}void @empty_outer( +extern "C" void empty_outer() { + int I = -1; + int J = -1; + int Zero = 0; +#pragma omp flatten + for (I = 0; I < Zero; ++I) + for (J = 0; J < 3; ++J) + body(I, J); +} + #endif diff --git a/openmp/runtime/test/transform/flatten/final-values.c b/openmp/runtime/test/transform/flatten/final-values.c index d770cf0a1fa2c8..ed605e9849950a 100644 --- a/openmp/runtime/test/transform/flatten/final-values.c +++ b/openmp/runtime/test/transform/flatten/final-values.c @@ -33,9 +33,46 @@ int main(void) { ; printf("empty-inner-i=%d empty-inner-j=%d\n", i, j); + + // An empty outer loop still runs its init, but never enters the body, so the + // inner init does not run and 'j' keeps the value from before the nest. + i = -1; + j = -1; +#pragma omp flatten + for (i = 0; i < Zero; ++i) + for (j = 0; j < 3; ++j) + ; + + printf("empty-outer-i=%d empty-outer-j=%d\n", i, j); + + // The middle loop is empty, so the innermost init never runs. + int k = -1; + i = -1; + j = -1; +#pragma omp flatten depth(3) + for (i = 0; i < 2; ++i) + for (j = 0; j < Zero; ++j) + for (k = 0; k < 3; ++k) + ; + + printf("mid-empty-i=%d mid-empty-j=%d mid-empty-k=%d\n", i, j, k); + + // Outer init is a DeclStmt (always run). The inner assignment is still + // skipped when that outer loop is empty. + j = -1; +#pragma omp flatten + for (int ii = 0; ii < Zero; ++ii) + for (j = 0; j < 3; ++j) + ; + + printf("decl-outer-j=%d\n", j); + return EXIT_SUCCESS; } // CHECK: i=2 j=3 // CHECK-NEXT: descending-i=-1 descending-j=1 // CHECK-NEXT: empty-inner-i=3 empty-inner-j=0 +// CHECK-NEXT: empty-outer-i=0 empty-outer-j=-1 +// CHECK-NEXT: mid-empty-i=2 mid-empty-j=0 mid-empty-k=-1 +// CHECK-NEXT: decl-outer-j=-1 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
