[flang-commits] [flang] [flang][PFT-to-MLIR] Do not wrap the DO of an OpenMP loop construct (PR #216280)
Kareem Ergawy via flang-commits
flang-commits at lists.llvm.org
Fri Aug 14 01:47:19 PDT 2026
https://github.com/ergawy created https://github.com/llvm/llvm-project/pull/216280
A DO attached to an OpenMP loop directive is driven directly into the loop op by OpenMPLoopConstruct lowering, so folding it into an scf.execute_region hides the iteration from that op and crashes lowering when the loop body is unstructured.
Extend the wrappability check with isOmpLoopBody(), the OpenMP analogue of the existing isAccLoopBody(): the immediate body DO, or one of the N associated iterator DOs under collapse(N) / ordered(N), is not wrappable. The shared parts of both checks -- collecting the enclosing DO chain, evaluating a clause's loop count, and the depth test -- are factored into helpers.
Co-authored-by: Claude
>From 5fb53e50fd150bc181cbdac674581f7feb94e914 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at gmail.com>
Date: Fri, 14 Aug 2026 01:37:10 -0700
Subject: [PATCH] [flang][PFT-to-MLIR] Do not wrap the DO of an OpenMP loop
construct
A DO attached to an OpenMP loop directive is driven directly into the
loop op by OpenMPLoopConstruct lowering, so folding it into an
scf.execute_region hides the iteration from that op and crashes lowering
when the loop body is unstructured.
Extend the wrappability check with isOmpLoopBody(), the OpenMP analogue
of the existing isAccLoopBody(): the immediate body DO, or one of the N
associated iterator DOs under collapse(N) / ordered(N), is not wrappable.
The shared parts of both checks -- collecting the enclosing DO chain,
evaluating a clause's loop count, and the depth test -- are factored into
helpers.
Co-authored-by: Claude Opus 5 (1M context) <noreply at anthropic.com>
---
flang/lib/Lower/PFTBuilder.cpp | 172 ++++++++++++------
.../OpenMP/wsloop-unstructured-cycle.f90 | 45 +++++
2 files changed, 157 insertions(+), 60 deletions(-)
create mode 100644 flang/test/Lower/OpenMP/wsloop-unstructured-cycle.f90
diff --git a/flang/lib/Lower/PFTBuilder.cpp b/flang/lib/Lower/PFTBuilder.cpp
index e5c91ac101679..1f381d3ffe19a 100644
--- a/flang/lib/Lower/PFTBuilder.cpp
+++ b/flang/lib/Lower/PFTBuilder.cpp
@@ -17,6 +17,7 @@
#include "llvm/ADT/IntervalMap.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
+#include <algorithm>
#include <limits>
#define DEBUG_TYPE "flang-pft"
@@ -2497,74 +2498,125 @@ hasIncomingBranch(const Fortran::lower::pft::Evaluation &construct) {
return walk(funit->evaluationList);
}
-/// True if \p eval is a DoConstruct driven directly into an enclosing acc.loop
-/// by the OpenACCLoopConstruct / OpenACCCombinedConstruct lowering — the
-/// immediate body DO, or one of the N collapsed iterator DOs reached by
-/// walking down from the body DO under a `collapse(N)` clause.
-static bool isAccLoopBody(const Fortran::lower::pft::Evaluation &eval) {
+/// The DoConstructs enclosing (and including) a DO evaluation; index k holds
+/// the one at depth k, so index 0 is the evaluation's own DO.
+using DoConstructChain = llvm::SmallVector<const parser::DoConstruct *, 4>;
+
+/// Value of \p intExpr, or INT64_MAX if it isn't a compile-time constant.
+static int64_t
+constantValueOrMax(const parser::ScalarIntConstantExpr &intExpr) {
+ if (const auto *expr = semantics::GetExpr(intExpr))
+ if (auto v = evaluate::ToInt64(*expr))
+ return *v;
+ return std::numeric_limits<int64_t>::max();
+}
+
+/// Fill \p chain with the DoConstruct at each depth above (and including)
+/// \p eval, and return the innermost enclosing evaluation that is not a
+/// DoConstruct — the one a directive would be attached to. Returns null if
+/// \p eval is not a DoConstruct or has no such enclosing evaluation.
+static const Fortran::lower::pft::Evaluation *
+collectEnclosingDoChain(const Fortran::lower::pft::Evaluation &eval,
+ DoConstructChain &chain) {
const auto *doConstruct = eval.getIf<parser::DoConstruct>();
if (!doConstruct)
- return false;
- // N from `collapse(N)`, or 1 if no clause. eval at depth d from the body
- // (d == 0 means eval IS the body) is a collapsed iterator iff d < N. If the
- // Collapse value isn't a compile-time constant, be conservative and treat
- // every DO in the chain as collapsed (INT64_MAX) — wrapping is opt-in and
- // a false "is collapsed" is safer than a false "is not".
- auto collapseN = [](const parser::AccClauseList &cl) -> int64_t {
- for (const parser::AccClause &c : cl.v)
- if (const auto *cc = std::get_if<parser::AccClause::Collapse>(&c.u)) {
- const auto &intExpr = std::get<parser::ScalarIntConstantExpr>(cc->v.t);
- if (const auto *expr = semantics::GetExpr(intExpr))
- if (auto v = evaluate::ToInt64(*expr))
- return *v;
- return std::numeric_limits<int64_t>::max();
- }
- return 1;
- };
-
- // candidates[k] is the DoConstruct at depth k above (and including) eval.
- llvm::SmallVector<const parser::DoConstruct *, 4> candidates{doConstruct};
+ return nullptr;
+ chain.push_back(doConstruct);
for (const Fortran::lower::pft::Evaluation *p = eval.parentConstruct; p;
p = p->parentConstruct) {
if (const auto *d = p->getIf<parser::DoConstruct>()) {
- candidates.push_back(d);
+ chain.push_back(d);
continue;
}
+ return p;
+ }
+ return nullptr;
+}
- if (const auto *acc = p->getIf<parser::OpenACCConstruct>()) {
- const parser::DoConstruct *body = nullptr;
- int64_t n = 1;
- if (const auto *loop =
- std::get_if<parser::OpenACCLoopConstruct>(&acc->u)) {
- if (const auto &b =
- std::get<std::optional<parser::DoConstruct>>(loop->t))
- body = &b.value();
- n = collapseN(std::get<parser::AccClauseList>(std::get<0>(loop->t).t));
- } else if (const auto *comb =
- std::get_if<parser::OpenACCCombinedConstruct>(&acc->u)) {
- if (const auto &b =
- std::get<std::optional<parser::DoConstruct>>(comb->t))
- body = &b.value();
- n = collapseN(std::get<parser::AccClauseList>(std::get<0>(comb->t).t));
- }
+/// True if the DO at depth 0 of \p chain is one of the \p n loops a directive
+/// associates with itself, given that the directive's body DO is \p body.
+///
+/// \p body is the outermost candidate, so the evaluation sits at depth
+/// `index of body in chain` below it and is associated iff that depth < \p n.
+/// A \p body outside \p chain is not an error: OpenMPLoopConstruct's body is
+/// found by searching the construct's block (looking through a BLOCK
+/// construct), so it can name a DO that is not on this ancestor chain.
+static bool isAssociatedLoop(const DoConstructChain &chain,
+ const parser::DoConstruct *body, int64_t n) {
+ if (!body)
+ return false;
+ auto it = llvm::find(chain, body);
+ if (it == chain.end())
+ return false;
+ return std::distance(chain.begin(), it) < n;
+}
- if (body) {
- // body is at index `candidates.size()-1` (the outermost candidate);
- // eval at depth (candidates.size()-1) from body. Collapsed iff < N.
- auto it = llvm::find(candidates, body);
- if (it != candidates.end()) {
- int64_t depth = std::distance(candidates.begin(), it);
- if (depth < n)
- return true;
- }
- }
- }
+/// True if \p eval is a DoConstruct attached to an enclosing OpenACC loop.
+static bool isAccLoopBody(const Fortran::lower::pft::Evaluation &eval) {
+ DoConstructChain chain;
+ const Fortran::lower::pft::Evaluation *p =
+ collectEnclosingDoChain(eval, chain);
+ if (!p)
+ return false;
+
+ const auto *acc = p->getIf<parser::OpenACCConstruct>();
+ if (!acc)
+ return false;
- break;
+ // N from `collapse(N)`, or 1 if no clause.
+ auto collapseValue = [](const parser::AccClauseList &cl) -> int64_t {
+ for (const parser::AccClause &c : cl.v)
+ if (const auto *cc = std::get_if<parser::AccClause::Collapse>(&c.u))
+ return constantValueOrMax(
+ std::get<parser::ScalarIntConstantExpr>(cc->v.t));
+ return 1;
+ };
+
+ const parser::DoConstruct *body = nullptr;
+ int64_t n = 1;
+ if (const auto *loop = std::get_if<parser::OpenACCLoopConstruct>(&acc->u)) {
+ if (const auto &b = std::get<std::optional<parser::DoConstruct>>(loop->t))
+ body = &b.value();
+ n = collapseValue(std::get<parser::AccClauseList>(std::get<0>(loop->t).t));
+ } else if (const auto *comb =
+ std::get_if<parser::OpenACCCombinedConstruct>(&acc->u)) {
+ if (const auto &b = std::get<std::optional<parser::DoConstruct>>(comb->t))
+ body = &b.value();
+ n = collapseValue(std::get<parser::AccClauseList>(std::get<0>(comb->t).t));
}
- return false;
+ return isAssociatedLoop(chain, body, n);
+}
+
+/// True if \p eval is a DoConstruct attached to an enclosing OpenMP loop.
+static bool isOmpLoopBody(const Fortran::lower::pft::Evaluation &eval) {
+ DoConstructChain chain;
+ const Fortran::lower::pft::Evaluation *p =
+ collectEnclosingDoChain(eval, chain);
+ if (!p)
+ return false;
+
+ const auto *omp = p->getIf<parser::OpenMPConstruct>();
+ if (!omp)
+ return false;
+
+ const auto *loop = std::get_if<parser::OpenMPLoopConstruct>(&omp->u);
+ if (!loop)
+ return false;
+
+ // Both `collapse(N)` and `ordered(N)` associate N loops with the directive,
+ // so the associated loop count is the larger of the two.
+ int64_t n = 1;
+ for (const parser::OmpClause &c : loop->BeginDir().Clauses().v) {
+ if (const auto *cc = std::get_if<parser::OmpClause::Collapse>(&c.u))
+ n = std::max(n, constantValueOrMax(cc->v));
+ else if (const auto *oc = std::get_if<parser::OmpClause::Ordered>(&c.u))
+ if (oc->v)
+ n = std::max(n, constantValueOrMax(*oc->v));
+ }
+
+ return isAssociatedLoop(chain, loop->GetNestedLoop(), n);
}
bool Fortran::lower::pft::isWrappableConstruct(
@@ -2581,10 +2633,10 @@ bool Fortran::lower::pft::isWrappableConstruct(
// Wrapping requires self-contained CFG.
//
- // Note: Loops attached to OpenACC constructs are not wrappable since
- // genOpenACCLoopFromDoConstruct takes over code-gen when a DoConstruct is
- // attached to an OpenACC directive. We might extend wrapping to such
- // unstructured loops later on if needed.
+ // Note: Loops attached to OpenACC/OpenMP constructs are not wrappable since
+ // the directive lowering (e.g. genOpenACCLoopFromDoConstruct) takes over
+ // code-gen when a DoConstruct is attached to such a directive. We might
+ // extend wrapping to such unstructured loops later on if needed.
return !hasUnwrappableInternals(eval) && !hasIncomingBranch(eval) &&
- !isAccLoopBody(eval);
+ !isAccLoopBody(eval) && !isOmpLoopBody(eval);
}
diff --git a/flang/test/Lower/OpenMP/wsloop-unstructured-cycle.f90 b/flang/test/Lower/OpenMP/wsloop-unstructured-cycle.f90
new file mode 100644
index 0000000000000..920a91d47c340
--- /dev/null
+++ b/flang/test/Lower/OpenMP/wsloop-unstructured-cycle.f90
@@ -0,0 +1,45 @@
+! RUN: bbc --wrap-unstructured-constructs-in-execute-region -emit-hlfir -fopenmp -o - %s | FileCheck %s --implicit-check-not=scf.execute_region
+
+! A DO attached to an OpenMP loop directive is lowered by the directive's own
+! code-gen, which takes the loop over. Such a DO must never be folded into an
+! scf.execute_region, even when wrapping is enabled and the loop is
+! unstructured -- here the IF-guarded CYCLE makes it so. The body's blocks
+! stay flat inside omp.loop_nest.
+!
+! --implicit-check-not on the RUN line asserts that no wrapping takes place
+! anywhere in the output.
+
+subroutine repro_final(x, y, n)
+ implicit none
+ integer n
+ double precision x(*), y(*)
+ integer i
+
+ !$omp do
+ do i = 1, n
+ if (x(i) > 0.0d0) then
+ y(1) = 0.0d0 ! any statement before CYCLE makes the loop unstructured
+ cycle
+ end if
+ y(2) = 1.0d0
+ end do
+ !$omp end do
+
+end subroutine repro_final
+
+! CHECK-LABEL: func.func @_QPrepro_final(
+! CHECK: omp.wsloop
+! CHECK: omp.loop_nest
+! CHECK: hlfir.assign
+! CHECK: cf.br ^bb[[TEST:[0-9]+]]
+! CHECK: ^bb[[TEST]]:
+! CHECK: arith.cmpf ogt
+! CHECK: cf.cond_br %{{[0-9]+}}, ^bb[[CYCLE:[0-9]+]], ^bb[[BODY:[0-9]+]]
+! CHECK: ^bb[[CYCLE]]:
+! CHECK: hlfir.assign
+! CHECK: cf.br ^bb[[EXIT:[0-9]+]]
+! CHECK: ^bb[[BODY]]:
+! CHECK: hlfir.assign
+! CHECK: cf.br ^bb[[EXIT]]
+! CHECK: ^bb[[EXIT]]:
+! CHECK: omp.yield
More information about the flang-commits
mailing list