https://github.com/zahiraam updated https://github.com/llvm/llvm-project/pull/222401
>From 4534a764eb60f89e4c38ca14e4b7a43f2e037f61 Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Wed, 9 Sep 2026 10:28:38 -0700 Subject: [PATCH 1/8] [OpenMP] Accept nested loop transformations as body of tile/stripe/reverse --- clang/lib/AST/StmtOpenMP.cpp | 18 +++-- .../OpenMP/tile_stripe_reverse_ast_print.cpp | 68 +++++++++++++++++++ 2 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 clang/test/OpenMP/tile_stripe_reverse_ast_print.cpp diff --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp index 44656266bcf08..a8d53c674b9cb 100644 --- a/clang/lib/AST/StmtOpenMP.cpp +++ b/clang/lib/AST/StmtOpenMP.cpp @@ -133,10 +133,12 @@ OMPLoopBasedDirective::tryToFindNextInnerLoop(Stmt *CurStmt, for (Stmt *S : CS->body()) { if (!S) continue; - // Peek past an OMPCanonicalLoop wrapper and/or an intra-tile hint to - // check whether this child is loop-like; keep the original (wrapped) - // node in CurStmt so the hint still reaches the loop-analysis + // Peek past a single-child container so a nested loop or + // loop-transformation directive (e.g. `#pragma omp reverse` inside + // the body of an outer loop) is recognized here. Preserve any + // intra-tile hint wrapper so it still reaches the loop-analysis // callback. + S = ignoreContainersKeepingIntraTileHint(S); Stmt *Inner = S; if (auto *CanonLoop = dyn_cast<OMPCanonicalLoop>(Inner)) Inner = CanonLoop->getLoopStmt(); @@ -152,7 +154,6 @@ OMPLoopBasedDirective::tryToFindNextInnerLoop(Stmt *CurStmt, CurStmt = S; continue; } - S = S->IgnoreContainers(); if (auto *InnerCS = dyn_cast_or_null<CompoundStmt>(S)) NextStatements.push_back(InnerCS); } @@ -177,6 +178,12 @@ bool OMPLoopBasedDirective::doForAllLoops( OnTransformationCallback) { CurStmt = ignoreContainersKeepingIntraTileHint(CurStmt); for (unsigned Cnt = 0; Cnt < NumLoops; ++Cnt) { + // If we peel a loop-transformation directive, the enclosing ForStmt is + // compiler-synthesized and its body may hold helper statements (e.g. + // `reverse` injects `.reversed.iv` and update exprs) before the next + // loop. Scan it as an imperfect nest so that e.g. `omp tile` followed by + // `omp reverse` is accepted. + bool PeeledTransformation = false; while (true) { auto *Dir = dyn_cast<OMPLoopTransformationDirective>(CurStmt); if (!Dir) @@ -202,6 +209,7 @@ bool OMPLoopBasedDirective::doForAllLoops( } CurStmt = TransformedStmt; + PeeledTransformation = true; } if (auto *CanonLoop = dyn_cast<OMPCanonicalLoop>(CurStmt)) CurStmt = CanonLoop->getLoopStmt(); @@ -224,7 +232,7 @@ bool OMPLoopBasedDirective::doForAllLoops( CurStmt = cast<CXXForRangeStmt>(LoopStmt)->getBody(); } CurStmt = OMPLoopBasedDirective::tryToFindNextInnerLoop( - CurStmt, TryImperfectlyNestedLoops); + CurStmt, TryImperfectlyNestedLoops || PeeledTransformation); } return true; } diff --git a/clang/test/OpenMP/tile_stripe_reverse_ast_print.cpp b/clang/test/OpenMP/tile_stripe_reverse_ast_print.cpp new file mode 100644 index 0000000000000..1eee82f241982 --- /dev/null +++ b/clang/test/OpenMP/tile_stripe_reverse_ast_print.cpp @@ -0,0 +1,68 @@ +// Check no warnings/errors +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=60 \ +// RUN: -fsyntax-only -verify %s +// expected-no-diagnostics + +// Check AST and unparsing +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=60 \ +// RUN: -ast-print %s | FileCheck %s --check-prefix=PRINT + +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=60 \ +// RUN: -emit-pch -o %t %s +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=60 \ +// RUN: -include-pch %t -ast-print %s | FileCheck %s --check-prefix=PRINT + +#ifndef HEADER +#define HEADER + +extern "C" void body(...); + +// PRINT-LABEL: void tile_then_reverse( +void tile_then_reverse() { + // PRINT: #pragma omp tile sizes(2, 2) + // PRINT: #pragma omp reverse +#pragma omp tile sizes(2, 2) +#pragma omp reverse + for (int i = 0; i < 20; ++i) + for (int j = 0; j < 20; ++j) + body(i, j); +} + +// PRINT-LABEL: void stripe_then_reverse( +void stripe_then_reverse() { + // PRINT: #pragma omp stripe sizes(2, 2) + // PRINT: #pragma omp reverse +#pragma omp stripe sizes(2, 2) +#pragma omp reverse + for (int i = 0; i < 20; ++i) + for (int j = 0; j < 20; ++j) + body(i, j); +} + +// PRINT-LABEL: void tile_over_inner_reverse( +void tile_over_inner_reverse() { + // PRINT: #pragma omp tile sizes(2, 2) +#pragma omp tile sizes(2, 2) +#pragma omp reverse + for (int j = 0; j < 20; ++j) { + // PRINT: #pragma omp reverse +#pragma omp reverse + for (int i = 0; i < 20; ++i) + body(j, i); + } +} + +// PRINT-LABEL: void stripe_over_inner_reverse( +void stripe_over_inner_reverse() { + // PRINT: #pragma omp stripe sizes(2, 2) +#pragma omp stripe sizes(2, 2) +#pragma omp reverse + for (int j = 0; j < 20; ++j) { + // PRINT: #pragma omp reverse +#pragma omp reverse + for (int i = 0; i < 20; ++i) + body(j, i); + } +} + +#endif >From 910f2617eb41e1fd82c4d5d76ec08eacb5375e51 Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Tue, 15 Sep 2026 11:01:11 -0700 Subject: [PATCH 2/8] Added test --- clang/test/OpenMP/tile_stripe_reverse_ast_print.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/clang/test/OpenMP/tile_stripe_reverse_ast_print.cpp b/clang/test/OpenMP/tile_stripe_reverse_ast_print.cpp index 1eee82f241982..5eee992f50483 100644 --- a/clang/test/OpenMP/tile_stripe_reverse_ast_print.cpp +++ b/clang/test/OpenMP/tile_stripe_reverse_ast_print.cpp @@ -65,4 +65,15 @@ void stripe_over_inner_reverse() { } } +// PRINT-LABEL: void for_collapse_then_reverse( +void for_collapse_then_reverse() { + // PRINT: #pragma omp for collapse(2) + // PRINT: #pragma omp reverse +#pragma omp for collapse(2) +#pragma omp reverse + for (int j = 0; j < 20; j += 2) + for (int i = 0; i < 20; i += 3) + body(j, i); +} + #endif >From 19511f5012f677632a80d4dcdc5f9a005a5aed5d Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Fri, 18 Sep 2026 05:43:06 -0700 Subject: [PATCH 3/8] Applied suggestion --- clang/lib/AST/StmtOpenMP.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp index a8d53c674b9cb..1b1db07805842 100644 --- a/clang/lib/AST/StmtOpenMP.cpp +++ b/clang/lib/AST/StmtOpenMP.cpp @@ -181,7 +181,7 @@ bool OMPLoopBasedDirective::doForAllLoops( // If we peel a loop-transformation directive, the enclosing ForStmt is // compiler-synthesized and its body may hold helper statements (e.g. // `reverse` injects `.reversed.iv` and update exprs) before the next - // loop. Scan it as an imperfect nest so that e.g. `omp tile` followed by + // loop. Scan it as an imperfect nest so that, e.g., `omp tile` followed by // `omp reverse` is accepted. bool PeeledTransformation = false; while (true) { >From 950467de2983e91fd5a93ac13c520ec41439b5d6 Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Thu, 24 Sep 2026 10:28:37 -0700 Subject: [PATCH 4/8] Applied suggestion --- clang/lib/AST/StmtOpenMP.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp index 1b1db07805842..229311676ffaa 100644 --- a/clang/lib/AST/StmtOpenMP.cpp +++ b/clang/lib/AST/StmtOpenMP.cpp @@ -154,6 +154,7 @@ OMPLoopBasedDirective::tryToFindNextInnerLoop(Stmt *CurStmt, CurStmt = S; continue; } + S = OMPLoopBasedDirective::ignoreIntraTileHint(S); if (auto *InnerCS = dyn_cast_or_null<CompoundStmt>(S)) NextStatements.push_back(InnerCS); } >From b2ebc40c40ffdea1d8907071c236ecb9a161ede7 Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Fri, 25 Sep 2026 12:04:10 -0700 Subject: [PATCH 5/8] Fixed build errors --- clang/include/clang/AST/StmtOpenMP.h | 37 +++++++++++++++++++--------- clang/lib/AST/StmtOpenMP.cpp | 13 +++++++--- clang/lib/Sema/SemaOpenMP.cpp | 21 +++++++++++----- 3 files changed, 49 insertions(+), 22 deletions(-) diff --git a/clang/include/clang/AST/StmtOpenMP.h b/clang/include/clang/AST/StmtOpenMP.h index 773e1153ccdfc..aed0415b77812 100644 --- a/clang/include/clang/AST/StmtOpenMP.h +++ b/clang/include/clang/AST/StmtOpenMP.h @@ -913,6 +913,12 @@ class OMPLoopBasedDirective : public OMPExecutableDirective { /// nullptr if the loop has no such hint. Callers that need the hint (see /// checkOpenMPIterationSpace) can peel \p HintWrapper themselves; everyone /// else can ignore it. + /// + /// \p RelaxNestForPeeledTransformation opts into treating the body of a + /// peeled loop-transformation directive as an imperfectly nested loop, so + /// e.g. `omp tile` accepts a nested `omp reverse` even though the transform + /// injects helper statements around the inner loop. Callers that require a + /// strict for-loop nest after peeling (e.g. `omp flatten`) leave it false. static bool doForAllLoops(Stmt *CurStmt, bool TryImperfectlyNestedLoops, unsigned NumLoops, @@ -920,25 +926,29 @@ class OMPLoopBasedDirective : public OMPExecutableDirective { Stmt * /*HintWrapper*/)> Callback, llvm::function_ref<void(OMPLoopTransformationDirective *)> - OnTransformationCallback); + OnTransformationCallback, + bool RelaxNestForPeeledTransformation = false); static bool doForAllLoops(Stmt *CurStmt, bool TryImperfectlyNestedLoops, unsigned NumLoops, llvm::function_ref<bool(unsigned, Stmt *)> Callback, llvm::function_ref<void(OMPLoopTransformationDirective *)> - OnTransformationCallback) { + OnTransformationCallback, + bool RelaxNestForPeeledTransformation = false) { auto &&NewCallback = [Callback](unsigned Cnt, Stmt *Loop, Stmt *) { return Callback(Cnt, Loop); }; return doForAllLoops(CurStmt, TryImperfectlyNestedLoops, NumLoops, - NewCallback, OnTransformationCallback); + NewCallback, OnTransformationCallback, + RelaxNestForPeeledTransformation); } static bool doForAllLoops(const Stmt *CurStmt, bool TryImperfectlyNestedLoops, unsigned NumLoops, llvm::function_ref<bool(unsigned, const Stmt *)> Callback, llvm::function_ref<void(const OMPLoopTransformationDirective *)> - OnTransformationCallback) { + OnTransformationCallback, + bool RelaxNestForPeeledTransformation = false) { auto &&NewCallback = [Callback](unsigned Cnt, Stmt *CurStmt) { return Callback(Cnt, CurStmt); }; @@ -947,28 +957,31 @@ class OMPLoopBasedDirective : public OMPExecutableDirective { OnTransformationCallback(A); }; return doForAllLoops(const_cast<Stmt *>(CurStmt), TryImperfectlyNestedLoops, - NumLoops, NewCallback, NewTransformCb); + NumLoops, NewCallback, NewTransformCb, + RelaxNestForPeeledTransformation); } /// Calls the specified callback function for all the loops in \p CurStmt, /// from the outermost to the innermost. - static bool - doForAllLoops(Stmt *CurStmt, bool TryImperfectlyNestedLoops, - unsigned NumLoops, - llvm::function_ref<bool(unsigned, Stmt *)> Callback) { + static bool doForAllLoops(Stmt *CurStmt, bool TryImperfectlyNestedLoops, + unsigned NumLoops, + llvm::function_ref<bool(unsigned, Stmt *)> Callback, + bool RelaxNestForPeeledTransformation = false) { auto &&TransformCb = [](OMPLoopTransformationDirective *) {}; return doForAllLoops(CurStmt, TryImperfectlyNestedLoops, NumLoops, Callback, - TransformCb); + TransformCb, RelaxNestForPeeledTransformation); } static bool doForAllLoops(const Stmt *CurStmt, bool TryImperfectlyNestedLoops, unsigned NumLoops, - llvm::function_ref<bool(unsigned, const Stmt *)> Callback) { + llvm::function_ref<bool(unsigned, const Stmt *)> Callback, + bool RelaxNestForPeeledTransformation = false) { auto &&NewCallback = [Callback](unsigned Cnt, const Stmt *CurStmt) { return Callback(Cnt, CurStmt); }; return doForAllLoops(const_cast<Stmt *>(CurStmt), TryImperfectlyNestedLoops, - NumLoops, NewCallback); + NumLoops, NewCallback, + RelaxNestForPeeledTransformation); } /// Calls the specified callback function for all the loop bodies in \p diff --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp index 8c9bb94fc5e81..8f994c89d103d 100644 --- a/clang/lib/AST/StmtOpenMP.cpp +++ b/clang/lib/AST/StmtOpenMP.cpp @@ -176,14 +176,17 @@ bool OMPLoopBasedDirective::doForAllLoops( Stmt *CurStmt, bool TryImperfectlyNestedLoops, unsigned NumLoops, llvm::function_ref<bool(unsigned, Stmt *, Stmt *)> Callback, llvm::function_ref<void(OMPLoopTransformationDirective *)> - OnTransformationCallback) { + OnTransformationCallback, + bool RelaxNestForPeeledTransformation) { CurStmt = ignoreContainersKeepingIntraTileHint(CurStmt); for (unsigned Cnt = 0; Cnt < NumLoops; ++Cnt) { // If we peel a loop-transformation directive, the enclosing ForStmt is // compiler-synthesized and its body may hold helper statements (e.g. // `reverse` injects `.reversed.iv` and update exprs) before the next - // loop. Scan it as an imperfect nest so that, e.g., `omp tile` followed by - // `omp reverse` is accepted. + // loop. When the caller opts in via RelaxNestForPeeledTransformation, scan + // it as an imperfect nest so that, e.g., `omp tile` followed by + // `omp reverse` is accepted. Callers that require a perfect for-loop nest + // after peeling (e.g. `omp flatten`) leave the flag false. bool PeeledTransformation = false; while (true) { auto *Dir = dyn_cast<OMPLoopTransformationDirective>(CurStmt); @@ -233,7 +236,9 @@ bool OMPLoopBasedDirective::doForAllLoops( CurStmt = cast<CXXForRangeStmt>(LoopStmt)->getBody(); } CurStmt = OMPLoopBasedDirective::tryToFindNextInnerLoop( - CurStmt, TryImperfectlyNestedLoops || PeeledTransformation); + CurStmt, + TryImperfectlyNestedLoops || + (RelaxNestForPeeledTransformation && PeeledTransformation)); } return true; } diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index 3bd68a9ace981..25aab7c2a20b1 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -15347,6 +15347,11 @@ bool SemaOpenMP::checkTransformableLoopNest( SmallVectorImpl<OMPLoopBasedDirective::HelperExprs> &LoopHelpers, Stmt *&Body, SmallVectorImpl<SmallVector<Stmt *>> &OriginalInits) { OriginalInits.emplace_back(); + // tile/stripe/reverse accept a nested loop-transformation directive as + // their associated loop; other loop-transforming constructs (unroll, split, + // interchange, flatten) require a strict for-loop nest. + bool RelaxNestForPeeledTransformation = + Kind == OMPD_tile || Kind == OMPD_stripe || Kind == OMPD_reverse; bool Result = OMPLoopBasedDirective::doForAllLoops( AStmt->IgnoreContainers(), /*TryImperfectlyNestedLoops=*/false, NumLoops, [this, &LoopHelpers, &Body, &OriginalInits, @@ -15382,7 +15387,8 @@ bool SemaOpenMP::checkTransformableLoopNest( }, [&OriginalInits](OMPLoopTransformationDirective *Transform) { updatePreInits(Transform, OriginalInits.back()); - }); + }, + RelaxNestForPeeledTransformation); assert(OriginalInits.back().empty() && "No preinit after innermost loop"); OriginalInits.pop_back(); return Result; @@ -15684,7 +15690,8 @@ static void addLoopPreInits(ASTContext &Context, /// Collect the loop statements (ForStmt or CXXRangeForStmt) of the affected /// loop of a construct. -static void collectLoopStmts(Stmt *AStmt, MutableArrayRef<Stmt *> LoopStmts) { +static void collectLoopStmts(Stmt *AStmt, MutableArrayRef<Stmt *> LoopStmts, + bool RelaxNestForPeeledTransformation = false) { size_t NumLoops = LoopStmts.size(); OMPLoopBasedDirective::doForAllLoops( AStmt, /*TryImperfectlyNestedLoops=*/false, NumLoops, @@ -15692,7 +15699,8 @@ static void collectLoopStmts(Stmt *AStmt, MutableArrayRef<Stmt *> LoopStmts) { assert(!LoopStmts[Cnt] && "Loop statement must not yet be assigned"); LoopStmts[Cnt] = CurStmt; return false; - }); + }, + RelaxNestForPeeledTransformation); assert(!is_contained(LoopStmts, nullptr) && "Expecting a loop statement for each affected loop"); } @@ -15745,7 +15753,7 @@ StmtResult SemaOpenMP::ActOnOpenMPTileDirective(ArrayRef<OMPClause *> Clauses, // Collect all affected loop statements. SmallVector<Stmt *> LoopStmts(NumLoops, nullptr); - collectLoopStmts(AStmt, LoopStmts); + collectLoopStmts(AStmt, LoopStmts, /*RelaxNestForPeeledTransformation=*/true); SmallVector<Stmt *, 4> PreInits; CaptureVars CopyTransformer(SemaRef); @@ -16102,7 +16110,7 @@ StmtResult SemaOpenMP::ActOnOpenMPStripeDirective(ArrayRef<OMPClause *> Clauses, // Collect all affected loop statements. SmallVector<Stmt *> LoopStmts(NumLoops, nullptr); - collectLoopStmts(AStmt, LoopStmts); + collectLoopStmts(AStmt, LoopStmts, /*RelaxNestForPeeledTransformation=*/true); SmallVector<Stmt *, 4> PreInits; CaptureVars CopyTransformer(SemaRef); @@ -16633,7 +16641,8 @@ StmtResult SemaOpenMP::ActOnOpenMPReverseDirective(Stmt *AStmt, // Find the loop statement. Stmt *LoopStmt = nullptr; - collectLoopStmts(AStmt, {LoopStmt}); + collectLoopStmts(AStmt, {LoopStmt}, + /*RelaxNestForPeeledTransformation=*/true); // Determine the PreInit declarations. SmallVector<Stmt *> PreInits; >From 3254263a8ef9a464eae6a8a78e66de38d3ddf8c3 Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Mon, 5 Oct 2026 11:21:31 -0700 Subject: [PATCH 6/8] Applied suggestion --- clang/lib/AST/StmtOpenMP.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp index 8f994c89d103d..a05583c80649d 100644 --- a/clang/lib/AST/StmtOpenMP.cpp +++ b/clang/lib/AST/StmtOpenMP.cpp @@ -193,6 +193,13 @@ bool OMPLoopBasedDirective::doForAllLoops( if (!Dir) break; + // tile/stripe regenerate intermediate loop bodies and drop the helper + // statements reverse injects into the body of the loop it transforms; + // compose only when the reversed loop is the innermost one. + if (RelaxNestForPeeledTransformation && + isa<OMPReverseDirective>(Dir->getDirective()) && + Cnt + 1 < NumLoops) + break; OnTransformationCallback(Dir); Stmt *TransformedStmt = Dir->getTransformedStmt(); >From efd5f0ca48eb70a7e373f41f1ab2a6b4470f7164 Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Mon, 5 Oct 2026 11:35:05 -0700 Subject: [PATCH 7/8] Fix format --- clang/lib/AST/StmtOpenMP.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp index a05583c80649d..2358b92d13346 100644 --- a/clang/lib/AST/StmtOpenMP.cpp +++ b/clang/lib/AST/StmtOpenMP.cpp @@ -197,8 +197,7 @@ bool OMPLoopBasedDirective::doForAllLoops( // statements reverse injects into the body of the loop it transforms; // compose only when the reversed loop is the innermost one. if (RelaxNestForPeeledTransformation && - isa<OMPReverseDirective>(Dir->getDirective()) && - Cnt + 1 < NumLoops) + isa<OMPReverseDirective>(Dir->getDirective()) && Cnt + 1 < NumLoops) break; OnTransformationCallback(Dir); >From fe94c170b2c38f73092e390f14b5af828090397c Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Tue, 6 Oct 2026 09:03:03 -0700 Subject: [PATCH 8/8] Re-emit statement before nested loop --- clang/include/clang/AST/StmtOpenMP.h | 15 ++- clang/lib/AST/StmtOpenMP.cpp | 31 +++--- clang/lib/Sema/SemaOpenMP.cpp | 28 ++++-- clang/test/OpenMP/stripe_messages.cpp | 13 +++ clang/test/OpenMP/tile_messages.cpp | 13 +++ .../test/transform/tile/tile-stripe-reverse.c | 94 +++++++++++++++++++ 6 files changed, 167 insertions(+), 27 deletions(-) create mode 100644 openmp/runtime/test/transform/tile/tile-stripe-reverse.c diff --git a/clang/include/clang/AST/StmtOpenMP.h b/clang/include/clang/AST/StmtOpenMP.h index aed0415b77812..a070d97a6db1f 100644 --- a/clang/include/clang/AST/StmtOpenMP.h +++ b/clang/include/clang/AST/StmtOpenMP.h @@ -905,6 +905,10 @@ class OMPLoopBasedDirective : public OMPExecutableDirective { return ignoreIntraTileHint(const_cast<Stmt *>(S)); } + /// Returns the statements of \p Body that precede its last statement (the + /// nested loop), such as the user counter update `omp reverse` emits. + static ArrayRef<Stmt *> getStmtsBeforeNestedLoop(Stmt *Body); + /// Calls the specified callback function for all the loops in \p CurStmt, /// from the outermost to the innermost. /// @@ -914,11 +918,12 @@ class OMPLoopBasedDirective : public OMPExecutableDirective { /// checkOpenMPIterationSpace) can peel \p HintWrapper themselves; everyone /// else can ignore it. /// - /// \p RelaxNestForPeeledTransformation opts into treating the body of a - /// peeled loop-transformation directive as an imperfectly nested loop, so - /// e.g. `omp tile` accepts a nested `omp reverse` even though the transform - /// injects helper statements around the inner loop. Callers that require a - /// strict for-loop nest after peeling (e.g. `omp flatten`) leave it false. + /// \p RelaxNestForPeeledTransformation allows a loop generated by a peeled + /// loop-transformation directive to hold statements before the next loop, + /// such as the user counter update `omp reverse` emits, so e.g. `omp tile` + /// accepts a nested `omp reverse` on a non-innermost loop. Only set it if the + /// caller re-emits those statements in the loops it generates; otherwise they + /// are dropped. static bool doForAllLoops(Stmt *CurStmt, bool TryImperfectlyNestedLoops, unsigned NumLoops, diff --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp index 2358b92d13346..5ba11b2d8c42f 100644 --- a/clang/lib/AST/StmtOpenMP.cpp +++ b/clang/lib/AST/StmtOpenMP.cpp @@ -34,6 +34,13 @@ Stmt *OMPLoopBasedDirective::ignoreIntraTileHint(Stmt *S) { return S; } +ArrayRef<Stmt *> OMPLoopBasedDirective::getStmtsBeforeNestedLoop(Stmt *Body) { + auto *CS = dyn_cast_or_null<CompoundStmt>(Body); + if (!CS || CS->size() < 2) + return {}; + return ArrayRef(CS->body_begin(), CS->size() - 1); +} + /// Like `Stmt::IgnoreContainers`, but stops at (and preserves) an intra-tile /// hint wrapper so that the enclosing loop walker can deliver the hint to /// loop-associated analysis (e.g. `collapse`). Non-hint attributed statements @@ -180,25 +187,16 @@ bool OMPLoopBasedDirective::doForAllLoops( bool RelaxNestForPeeledTransformation) { CurStmt = ignoreContainersKeepingIntraTileHint(CurStmt); for (unsigned Cnt = 0; Cnt < NumLoops; ++Cnt) { - // If we peel a loop-transformation directive, the enclosing ForStmt is - // compiler-synthesized and its body may hold helper statements (e.g. - // `reverse` injects `.reversed.iv` and update exprs) before the next - // loop. When the caller opts in via RelaxNestForPeeledTransformation, scan - // it as an imperfect nest so that, e.g., `omp tile` followed by - // `omp reverse` is accepted. Callers that require a perfect for-loop nest - // after peeling (e.g. `omp flatten`) leave the flag false. + // A loop generated by a peeled transformation may hold statements before + // the next loop (e.g. `reverse` recomputes the user counter there). They + // are not intervening user code, so callers that re-emit them may look + // through them; the rest of the nest is still checked as usual. bool PeeledTransformation = false; while (true) { auto *Dir = dyn_cast<OMPLoopTransformationDirective>(CurStmt); if (!Dir) break; - // tile/stripe regenerate intermediate loop bodies and drop the helper - // statements reverse injects into the body of the loop it transforms; - // compose only when the reversed loop is the innermost one. - if (RelaxNestForPeeledTransformation && - isa<OMPReverseDirective>(Dir->getDirective()) && Cnt + 1 < NumLoops) - break; OnTransformationCallback(Dir); Stmt *TransformedStmt = Dir->getTransformedStmt(); @@ -241,10 +239,11 @@ bool OMPLoopBasedDirective::doForAllLoops( "Expected canonical for or range-based for loops."); CurStmt = cast<CXXForRangeStmt>(LoopStmt)->getBody(); } + if (RelaxNestForPeeledTransformation && PeeledTransformation && + !getStmtsBeforeNestedLoop(CurStmt).empty()) + CurStmt = cast<CompoundStmt>(CurStmt)->body_back(); CurStmt = OMPLoopBasedDirective::tryToFindNextInnerLoop( - CurStmt, - TryImperfectlyNestedLoops || - (RelaxNestForPeeledTransformation && PeeledTransformation)); + CurStmt, TryImperfectlyNestedLoops); } return true; } diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index 25aab7c2a20b1..24610d8bdcb60 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -15347,11 +15347,11 @@ bool SemaOpenMP::checkTransformableLoopNest( SmallVectorImpl<OMPLoopBasedDirective::HelperExprs> &LoopHelpers, Stmt *&Body, SmallVectorImpl<SmallVector<Stmt *>> &OriginalInits) { OriginalInits.emplace_back(); - // tile/stripe/reverse accept a nested loop-transformation directive as - // their associated loop; other loop-transforming constructs (unroll, split, - // interchange, flatten) require a strict for-loop nest. + // Only tile and stripe re-emit the statements before the nested loop in a + // nested transformation's generated loop (see appendStmtsBeforeNestedLoop); + // other constructs would silently drop them. bool RelaxNestForPeeledTransformation = - Kind == OMPD_tile || Kind == OMPD_stripe || Kind == OMPD_reverse; + Kind == OMPD_tile || Kind == OMPD_stripe; bool Result = OMPLoopBasedDirective::doForAllLoops( AStmt->IgnoreContainers(), /*TryImperfectlyNestedLoops=*/false, NumLoops, [this, &LoopHelpers, &Body, &OriginalInits, @@ -15705,6 +15705,19 @@ static void collectLoopStmts(Stmt *AStmt, MutableArrayRef<Stmt *> LoopStmts, "Expecting a loop statement for each affected loop"); } +/// A non-innermost affected loop generated by a nested loop transformation +/// (e.g. `omp reverse`) computes the user counter in statements ahead of the +/// next loop. A construct that rebuilds the nest only keeps the innermost +/// body, so it must re-emit these statements in the matching loop. +static void appendStmtsBeforeNestedLoop(Stmt *LoopStmt, + SmallVectorImpl<Stmt *> &BodyParts) { + Stmt *Body = isa<ForStmt>(LoopStmt) + ? cast<ForStmt>(LoopStmt)->getBody() + : cast<CXXForRangeStmt>(LoopStmt)->getBody(); + llvm::append_range(BodyParts, + OMPLoopBasedDirective::getStmtsBeforeNestedLoop(Body)); +} + /// Build and return a DeclRefExpr for the floor induction variable using the /// SemaRef and the provided parameters. static Expr *makeFloorIVRef(Sema &SemaRef, ArrayRef<VarDecl *> FloorIndVars, @@ -16002,6 +16015,8 @@ StmtResult SemaOpenMP::ActOnOpenMPTileDirective(ArrayRef<OMPClause *> Clauses, BodyParts.append(LoopHelper.Updates.begin(), LoopHelper.Updates.end()); if (auto *SourceCXXFor = dyn_cast<CXXForRangeStmt>(LoopStmt)) BodyParts.push_back(SourceCXXFor->getLoopVarStmt()); + if (static_cast<unsigned>(I) + 1 < NumLoops) + appendStmtsBeforeNestedLoop(LoopStmt, BodyParts); BodyParts.push_back(Inner); Inner = CompoundStmt::Create(Context, BodyParts, FPOptionsOverride(), Inner->getBeginLoc(), Inner->getEndLoc()); @@ -16272,6 +16287,8 @@ StmtResult SemaOpenMP::ActOnOpenMPStripeDirective(ArrayRef<OMPClause *> Clauses, BodyParts.append(LoopHelper.Updates.begin(), LoopHelper.Updates.end()); if (auto *SourceCXXFor = dyn_cast<CXXForRangeStmt>(LoopStmt)) BodyParts.push_back(SourceCXXFor->getLoopVarStmt()); + if (static_cast<unsigned>(I) + 1 < NumLoops) + appendStmtsBeforeNestedLoop(LoopStmt, BodyParts); BodyParts.push_back(Inner); Inner = CompoundStmt::Create(Context, BodyParts, FPOptionsOverride(), Inner->getBeginLoc(), Inner->getEndLoc()); @@ -16641,8 +16658,7 @@ StmtResult SemaOpenMP::ActOnOpenMPReverseDirective(Stmt *AStmt, // Find the loop statement. Stmt *LoopStmt = nullptr; - collectLoopStmts(AStmt, {LoopStmt}, - /*RelaxNestForPeeledTransformation=*/true); + collectLoopStmts(AStmt, {LoopStmt}); // Determine the PreInit declarations. SmallVector<Stmt *> PreInits; diff --git a/clang/test/OpenMP/stripe_messages.cpp b/clang/test/OpenMP/stripe_messages.cpp index 3f171f40d71d9..2e4675c35cc6c 100644 --- a/clang/test/OpenMP/stripe_messages.cpp +++ b/clang/test/OpenMP/stripe_messages.cpp @@ -169,3 +169,16 @@ void f() { ; } } // namespace GH139433 + +// The loop generated by 'reverse' may be a non-innermost affected loop, but +// user code between it and the next affected loop is still intervening code. +void stripe_of_reverse_intervening() { + // expected-error@+3 {{statement after '#pragma omp stripe' must be a for loop}} +#pragma omp stripe sizes(2, 2) +#pragma omp reverse + for (int i = 0; i < 7; ++i) { + int k = 3; + for (int j = 0; j < 7; ++j) + ; + } +} diff --git a/clang/test/OpenMP/tile_messages.cpp b/clang/test/OpenMP/tile_messages.cpp index 021262ac9a54a..72ceef71566d6 100644 --- a/clang/test/OpenMP/tile_messages.cpp +++ b/clang/test/OpenMP/tile_messages.cpp @@ -210,3 +210,16 @@ void unroll_of_tile() { ; } } // namespace consumed_by_transformation + +// The loop generated by 'reverse' may be a non-innermost affected loop, but +// user code between it and the next affected loop is still intervening code. +void tile_of_reverse_intervening() { + // expected-error@+3 {{statement after '#pragma omp tile' must be a for loop}} +#pragma omp tile sizes(2, 2) +#pragma omp reverse + for (int i = 0; i < 7; ++i) { + int k = 3; + for (int j = 0; j < 7; ++j) + ; + } +} diff --git a/openmp/runtime/test/transform/tile/tile-stripe-reverse.c b/openmp/runtime/test/transform/tile/tile-stripe-reverse.c new file mode 100644 index 0000000000000..7f599737b1e8a --- /dev/null +++ b/openmp/runtime/test/transform/tile/tile-stripe-reverse.c @@ -0,0 +1,94 @@ +// RUN: %libomp-compile -fopenmp-version=60 && %libomp-run \ +// RUN: | FileCheck %s --match-full-lines + +// tile/stripe over a reverse that is not the innermost affected loop must +// keep the reversal (regression test for #222401). + +#ifndef HEADER +#define HEADER + +#include <stdio.h> +#include <stdlib.h> + +int main() { + printf("tile\n"); +#pragma omp tile sizes(2, 2) +#pragma omp reverse + for (int i = 0; i < 4; ++i) + for (int j = 0; j < 4; ++j) + printf("i=%d j=%d\n", i, j); + + printf("stripe\n"); +#pragma omp stripe sizes(2, 2) +#pragma omp reverse + for (int i = 0; i < 4; ++i) + for (int j = 0; j < 4; ++j) + printf("i=%d j=%d\n", i, j); + + printf("nested\n"); +#pragma omp tile sizes(2, 2) +#pragma omp reverse + for (int j = 0; j < 4; ++j) { +#pragma omp reverse + for (int i = 0; i < 4; ++i) + printf("j=%d i=%d\n", j, i); + } + + printf("done\n"); + return EXIT_SUCCESS; +} + +#endif /* HEADER */ + +// CHECK: tile +// CHECK-NEXT: i=3 j=0 +// CHECK-NEXT: i=3 j=1 +// CHECK-NEXT: i=2 j=0 +// CHECK-NEXT: i=2 j=1 +// CHECK-NEXT: i=3 j=2 +// CHECK-NEXT: i=3 j=3 +// CHECK-NEXT: i=2 j=2 +// CHECK-NEXT: i=2 j=3 +// CHECK-NEXT: i=1 j=0 +// CHECK-NEXT: i=1 j=1 +// CHECK-NEXT: i=0 j=0 +// CHECK-NEXT: i=0 j=1 +// CHECK-NEXT: i=1 j=2 +// CHECK-NEXT: i=1 j=3 +// CHECK-NEXT: i=0 j=2 +// CHECK-NEXT: i=0 j=3 +// CHECK-NEXT: stripe +// CHECK-NEXT: i=3 j=0 +// CHECK-NEXT: i=3 j=1 +// CHECK-NEXT: i=2 j=0 +// CHECK-NEXT: i=2 j=1 +// CHECK-NEXT: i=3 j=2 +// CHECK-NEXT: i=3 j=3 +// CHECK-NEXT: i=2 j=2 +// CHECK-NEXT: i=2 j=3 +// CHECK-NEXT: i=1 j=0 +// CHECK-NEXT: i=1 j=1 +// CHECK-NEXT: i=0 j=0 +// CHECK-NEXT: i=0 j=1 +// CHECK-NEXT: i=1 j=2 +// CHECK-NEXT: i=1 j=3 +// CHECK-NEXT: i=0 j=2 +// CHECK-NEXT: i=0 j=3 +// CHECK-NEXT: nested +// CHECK-NEXT: j=3 i=3 +// CHECK-NEXT: j=3 i=2 +// CHECK-NEXT: j=2 i=3 +// CHECK-NEXT: j=2 i=2 +// CHECK-NEXT: j=3 i=1 +// CHECK-NEXT: j=3 i=0 +// CHECK-NEXT: j=2 i=1 +// CHECK-NEXT: j=2 i=0 +// CHECK-NEXT: j=1 i=3 +// CHECK-NEXT: j=1 i=2 +// CHECK-NEXT: j=0 i=3 +// CHECK-NEXT: j=0 i=2 +// CHECK-NEXT: j=1 i=1 +// CHECK-NEXT: j=1 i=0 +// CHECK-NEXT: j=0 i=1 +// CHECK-NEXT: j=0 i=0 +// CHECK-NEXT: done _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
