llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Zahira Ammarguellat (zahiraam)
<details>
<summary>Changes</summary>
`Clang` rejects `#pragma omp tile` or `#pragma omp stripe` immediately followed
by `#pragma omp reverse` on the same loop nest, emitting the error:
`error: statement after '#pragma omp tile' must be a for loop`
See https://godbolt.org/z/d1fsfaMe9
The correct behavior is to accept it. Bare`reverse`, bare `tile`/`stripe`, and
`tile` followed by `stripe` all work. Only `tile`/`stripe` and `reverse` fails.
This PR fixes the issue.
---
Full diff: https://github.com/llvm/llvm-project/pull/222401.diff
2 Files Affected:
- (modified) clang/lib/AST/StmtOpenMP.cpp (+13-5)
- (added) clang/test/OpenMP/tile_stripe_reverse_ast_print.cpp (+68)
``````````diff
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
``````````
</details>
https://github.com/llvm/llvm-project/pull/222401
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits