[Openmp-commits] [clang] [openmp] [OpenMP] Tile/stripe followed by reverse (PR #222401)
Zahira Ammarguellat via Openmp-commits
openmp-commits at lists.llvm.org
Wed Oct 7 04:12:58 PDT 2026
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 <Zahira.Ammarguellat at intel.com>
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 <Zahira.Ammarguellat at intel.com>
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 <Zahira.Ammarguellat at intel.com>
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 <Zahira.Ammarguellat at intel.com>
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 <Zahira.Ammarguellat at intel.com>
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 <Zahira.Ammarguellat at intel.com>
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 <Zahira.Ammarguellat at intel.com>
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 <Zahira.Ammarguellat at intel.com>
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 at +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 at +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
More information about the Openmp-commits
mailing list