[flang-commits] [flang] [flang] Rewrite an IF body ending in CYCLE as an IF/ELSE (PR #224718)
via flang-commits
flang-commits at lists.llvm.org
Fri Sep 18 12:50:16 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-openmp
@llvm/pr-subscribers-flang-fir-hlfir
Author: Vijay Kandiah (VijayKandiah)
<details>
<summary>Changes</summary>
A `CYCLE` marks the enclosing DO construct unstructured, which costs the structured form of the loop.
PFTBuilder already removes this branch for the simplest shape: `rewriteIfGotos` turns `if (cond) cycle` into a negated IF, but only when the `CYCLE` is the sole statement in the IF body (`evaluationList->size() == 3`). A leading statement defeats it, even though the result is the same loop. Currently:
if (cond) cycle -> structured
if (cond) then; a = 0; cycle; end if -> unstructured
if (cond) then; a = 0; else ...; end if -> structured
This patch generalizes the rewrite to an IF body whose *last* statement is a `CYCLE`. The condition cannot simply be negated, because the statements ahead of the `CYCLE` must still run when it holds, so the construct is reshaped into an IF/ELSE with the statements following it moved into a new ELSE branch:
<<IfConstruct>> <<IfConstruct>>
1 IfThenStmt: if(cond) then 1 IfThenStmt: if(cond) then
2 Statement: ... 2 Statement: ...
3 CycleStmt: cycle => * ElseStmt
4 EndIfStmt 5 Statement: ...
<<End IfConstruct>> 4 EndIfStmt
5 Statement: ... <<End IfConstruct>>
6 EndDoStmt 6 EndDoStmt
The rewrite in this change is kept narrow. The `CYCLE` must be the last statement of a plain IF-THEN body, unlabelled, and either unnamed or naming the enclosing DO. A `CYCLE` ending an ELSE or ELSE IF branch sits in the same position and is left alone, as is a `GOTO`, whose target is arbitrary.
### Effect
As an example, for this loop written with `CYCLE`:
!$acc parallel loop present(rho, v)
do i = 1, n
if (rho(i) <= 1.0d-10) then
v(i) = 0.0d0
cycle
end if
v(i) = 1.0d0 / rho(i)
end do
`bbc -fopenacc -emit-hlfir` previously emitted the loop with no bounds on the op, leaving the trip test and the latch as `cf` blocks in the body:
acc.loop combined(parallel) private(%27 : !fir.ref<i32>) {
...
cf.cond_br %33, ^bb2, ^bb6
...
and now emits the structured form, the same as the IF/ELSE spelling of the same loop:
acc.loop combined(parallel) private(%26 : !fir.ref<i32>) control(%arg3 : i32) = (%c1_i32 : i32) to (%25 : i32) step (%c1_i32_1 : i32) {
...
fir.if %32 {
...
---
Full diff: https://github.com/llvm/llvm-project/pull/224718.diff
7 Files Affected:
- (modified) flang/lib/Lower/PFTBuilder.cpp (+111-2)
- (modified) flang/test/Lower/OpenACC/Todo/acc-unstructured-combined-construct.f90 (+1-1)
- (modified) flang/test/Lower/OpenACC/Todo/acc-unstructured-loop-construct.f90 (+1-1)
- (added) flang/test/Lower/OpenACC/acc-loop-trailing-cycle.f90 (+29)
- (modified) flang/test/Lower/OpenACC/acc-unstructured.f90 (+15-18)
- (modified) flang/test/Lower/OpenMP/wsloop-unstructured-cycle.f90 (+18-9)
- (added) flang/test/Lower/if-cycle-to-if-else.f90 (+70)
``````````diff
diff --git a/flang/lib/Lower/PFTBuilder.cpp b/flang/lib/Lower/PFTBuilder.cpp
index 82c16169b8e422..4f2d228f3ea2fa 100644
--- a/flang/lib/Lower/PFTBuilder.cpp
+++ b/flang/lib/Lower/PFTBuilder.cpp
@@ -778,6 +778,9 @@ class PFTBuilder {
lower::pft::EvaluationList::iterator ifConstructIt;
parser::Label ifTargetLabel;
bool isCycleStmt = false;
+ // Statements precede the branch, so the IF is rewritten as an IF/ELSE
+ // rather than by negating its condition.
+ bool hasLeadingStmts = false;
};
llvm::SmallVector<T> ifCandidateStack;
const auto *doStmt =
@@ -810,7 +813,11 @@ class PFTBuilder {
ifCandidateStack.back().ifConstructIt;
lower::pft::EvaluationList::iterator successorIt =
std::next(ifConstructIt);
- if (successorIt != it) {
+ if (successorIt != it && ifCandidateStack.back().hasLeadingStmts) {
+ rewriteIfCycleAsIfElse(evaluationList,
+ *ifConstructIt->evaluationList,
+ ifConstructIt, successorIt, it, firstStmt);
+ } else if (successorIt != it) {
Fortran::lower::pft::EvaluationList &ifBodyList =
*ifConstructIt->evaluationList;
lower::pft::EvaluationList::iterator branchStmtIt =
@@ -843,13 +850,115 @@ class PFTBuilder {
std::string cycleName = getConstructName(*cycleStmt);
if (cycleName.empty() || cycleName == doName)
// This candidate will match doStmt's EndDoStmt.
- ifCandidateStack.push_back({it, {}, true});
+ ifCandidateStack.push_back({it, {}, /*isCycleStmt=*/true});
}
}
+ } else if (doStmt && eval.isA<parser::IfConstruct>() &&
+ eval.evaluationList->size() > 3) {
+ // An IF body whose last statement is a CYCLE.
+ lower::pft::EvaluationList &bodyList = *eval.evaluationList;
+ auto branchIt = std::prev(std::prev(bodyList.end()));
+ const auto *cycleStmt = branchIt->getIf<parser::CycleStmt>();
+ // A CYCLE ending an ELSE or ELSE IF branch sits in the same position
+ // but must not match.
+ bool hasElseBranch =
+ llvm::any_of(bodyList, [](const lower::pft::Evaluation &bodyEval) {
+ return bodyEval.isIntermediateConstructStmt();
+ });
+ if (cycleStmt && !hasElseBranch && !branchIt->label) {
+ std::string cycleName = getConstructName(*cycleStmt);
+ if (cycleName.empty() || cycleName == doName)
+ // This candidate will match doStmt's EndDoStmt.
+ ifCandidateStack.push_back({it, {}, /*isCycleStmt=*/true,
+ /*hasLeadingStmts=*/true});
+ }
}
}
}
+ /// Rewrite an IfConstruct whose body ends in a CycleStmt, with statements
+ /// ahead of it, as an IF/ELSE. The pre-branch-analysis code:
+ ///
+ /// <<IfConstruct>>
+ /// 1 IfThenStmt: if(cond) then
+ /// 2 Statement: ...
+ /// 3 CycleStmt: cycle
+ /// 4 EndIfStmt
+ /// <<End IfConstruct>>
+ /// 5 Statement: ...
+ /// 6 EndDoStmt
+ ///
+ /// becomes:
+ ///
+ /// <<IfConstruct>>
+ /// 1 IfThenStmt: if(cond) then
+ /// 2 Statement: ...
+ /// * ElseStmt
+ /// 5 Statement: ...
+ /// 4 EndIfStmt
+ /// <<End IfConstruct>>
+ /// 6 EndDoStmt
+ ///
+ /// When the branch is the whole IF body there is nothing to keep, so
+ /// rewriteIfGotos deletes the branch and negates the condition. Here
+ /// statement 2 must still run when the condition holds, so the condition is
+ /// left as written and the statements that followed the construct go into an
+ /// ELSE branch instead.
+ ///
+ /// The synthesized ElseStmt has no source position and no index of its own.
+ template <typename FirstStmtFn>
+ void
+ rewriteIfCycleAsIfElse(lower::pft::EvaluationList &evaluationList,
+ lower::pft::EvaluationList &ifBodyList,
+ lower::pft::EvaluationList::iterator ifConstructIt,
+ lower::pft::EvaluationList::iterator successorIt,
+ lower::pft::EvaluationList::iterator it,
+ FirstStmtFn firstStmt) {
+ lower::pft::EvaluationList::iterator branchStmtIt =
+ std::prev(std::prev(ifBodyList.end()));
+ assert(branchStmtIt->isA<parser::CycleStmt>() &&
+ "expected cycle statement");
+
+ // The last statement reachable in the THEN branch, which currently falls
+ // through to the CycleStmt. Descend through nested constructs to find it.
+ lower::pft::Evaluation *thenTail = &*std::prev(branchStmtIt);
+ while (thenTail->evaluationList && !thenTail->evaluationList->empty())
+ thenTail = &thenTail->evaluationList->back();
+ assert(thenTail->lexicalSuccessor == &*branchStmtIt &&
+ "expected fallthrough to the cycle statement");
+
+ ifBodyList.erase(branchStmtIt);
+ lower::pft::EvaluationList::iterator endIfStmtIt =
+ std::prev(ifBodyList.end());
+
+ // A shared node is enough: ElseStmt only wraps an optional construct name,
+ // which is absent here, and genFIR for it is a nop.
+ static const parser::ElseStmt elseStmt{std::optional<parser::Name>{}};
+ lower::pft::EvaluationList::iterator elseStmtIt = ifBodyList.emplace(
+ endIfStmtIt,
+ lower::pft::Evaluation{
+ elseStmt, lower::pft::PftNode{*ifConstructIt}, {}, {}});
+ elseStmtIt->parentConstruct = &*ifConstructIt;
+
+ // The statement preceding the branch target falls through to the EndIfStmt
+ // once it has been moved inside the construct.
+ lower::pft::Evaluation *movedTail = &*std::prev(it);
+ while (movedTail->evaluationList && !movedTail->evaluationList->empty())
+ movedTail = &movedTail->evaluationList->back();
+ assert(movedTail->lexicalSuccessor == firstStmt(&*it) &&
+ "expected fallthrough to the branch target");
+
+ thenTail->lexicalSuccessor = &*elseStmtIt;
+ elseStmtIt->lexicalSuccessor = firstStmt(&*successorIt);
+ movedTail->lexicalSuccessor = &*endIfStmtIt;
+ endIfStmtIt->lexicalSuccessor = firstStmt(&*it);
+
+ ifBodyList.splice(endIfStmtIt, evaluationList, successorIt, it);
+ for (lower::pft::EvaluationList::iterator movedIt = std::next(elseStmtIt);
+ movedIt != endIfStmtIt; ++movedIt)
+ movedIt->parentConstruct = &*ifConstructIt;
+ }
+
/// Mark IO statement ERR, EOR, and END specifier branch targets.
/// Mark an IO statement with an assigned format as unstructured.
template <typename A>
diff --git a/flang/test/Lower/OpenACC/Todo/acc-unstructured-combined-construct.f90 b/flang/test/Lower/OpenACC/Todo/acc-unstructured-combined-construct.f90
index 110940d9e5069b..d070019b858044 100644
--- a/flang/test/Lower/OpenACC/Todo/acc-unstructured-combined-construct.f90
+++ b/flang/test/Lower/OpenACC/Todo/acc-unstructured-combined-construct.f90
@@ -13,8 +13,8 @@ subroutine test_unstructured_collapse_cycle(a)
do j = 1, 8
do i = 1, 8
if (i == jdiag) then
+ if (j == jdiag) cycle
a(i, j) = 0.0d0
- cycle
end if
a(i, j) = real(i + j, 8)
end do
diff --git a/flang/test/Lower/OpenACC/Todo/acc-unstructured-loop-construct.f90 b/flang/test/Lower/OpenACC/Todo/acc-unstructured-loop-construct.f90
index f26e95d235d049..4a1af74f940a7d 100644
--- a/flang/test/Lower/OpenACC/Todo/acc-unstructured-loop-construct.f90
+++ b/flang/test/Lower/OpenACC/Todo/acc-unstructured-loop-construct.f90
@@ -80,8 +80,8 @@ subroutine test_unstructured_collapse_loop_only(a)
do j = 1, 8
do i = 1, 8
if (i == jdiag) then
+ if (j == jdiag) cycle
a(i, j) = 0.0d0
- cycle
end if
a(i, j) = real(i + j, 8)
end do
diff --git a/flang/test/Lower/OpenACC/acc-loop-trailing-cycle.f90 b/flang/test/Lower/OpenACC/acc-loop-trailing-cycle.f90
new file mode 100644
index 00000000000000..89f8d372bd2131
--- /dev/null
+++ b/flang/test/Lower/OpenACC/acc-loop-trailing-cycle.f90
@@ -0,0 +1,29 @@
+! An IF body ending in a CYCLE must still lower to an acc.loop with a control
+! clause: without the induction variable and bounds there is nothing to map
+! onto gangs and threads, so the loop could only be emitted as a serial kernel.
+
+! RUN: bbc -fopenacc -emit-hlfir %s -o - | FileCheck %s
+
+subroutine trailing_cycle(n, rho, v)
+ implicit none
+ integer, intent(in) :: n
+ real(8), intent(in) :: rho(n)
+ real(8), intent(out) :: v(n)
+ integer :: i
+
+ !$acc parallel loop present(rho, v)
+ do i = 1, n
+ if (rho(i) <= 1.0d-10) then
+ v(i) = 0.0d0
+ cycle
+ end if
+ v(i) = 1.0d0 / rho(i)
+ end do
+end subroutine trailing_cycle
+
+! CHECK-LABEL: func.func @_QPtrailing_cycle
+! CHECK: acc.parallel
+! CHECK: acc.loop
+! CHECK-SAME: control(%{{.*}} : i32) = (%{{.*}} : i32) to (%{{.*}} : i32) step (%{{.*}} : i32)
+! CHECK: fir.if
+! CHECK-NOT: cf.cond_br
diff --git a/flang/test/Lower/OpenACC/acc-unstructured.f90 b/flang/test/Lower/OpenACC/acc-unstructured.f90
index cbb27d74cc96c2..b2a17915a43e3a 100644
--- a/flang/test/Lower/OpenACC/acc-unstructured.f90
+++ b/flang/test/Lower/OpenACC/acc-unstructured.f90
@@ -249,10 +249,10 @@ subroutine test_unstructured8(a, n)
! CHECK: cf.cond_br
! Test that `acc serial loop collapse(N)` whose body has an early-exit
-! (here, `if (cond) then ... cycle ... end if`) lowers cleanly. The
-! corresponding acc.loop must privatize all N induction variables, carry
-! both `collapse = [N]` and `unstructured` attributes, and emit the
-! iteration mechanics for all N levels as explicit cf inside the body.
+! (here, `if (cond) then ... cycle ... end if`) lowers cleanly. The IF-guarded
+! CYCLE is restructured into an IF/ELSE by the PFT, so the loop stays
+! structured: acc.loop privatizes all N induction variables and carries their
+! bounds in control(...), with the body's conditional as a fir.if.
subroutine test_unstructured_collapse_cycle(a)
integer :: i, j, jdiag
real(8) :: a(:,:)
@@ -275,19 +275,14 @@ subroutine test_unstructured_collapse_cycle(a)
! Both induction variables (j and i) are privatized:
! CHECK: %[[PRIVJ:.*]] = acc.private varPtr(%{{.*}} : !fir.ref<i32>) recipe(@privatization_ref_i32) implicit(true) name("j") -> !fir.ref<i32>
! CHECK: %[[PRIVI:.*]] = acc.private varPtr(%{{.*}} : !fir.ref<i32>) recipe(@privatization_ref_i32) implicit(true) name("i") -> !fir.ref<i32>
-! No control(...) on acc.loop — bounds are not on the op:
-! CHECK: acc.loop combined(serial) private(%[[PRIVJ]], %[[PRIVI]] : !fir.ref<i32>, !fir.ref<i32>) {
-! Outer loop trip-count test (j) emitted as cf:
-! CHECK: arith.cmpi sgt
-! CHECK: cf.cond_br
-! Inner loop trip-count test (i) emitted as cf:
-! CHECK: arith.cmpi sgt
-! CHECK: cf.cond_br
-! The if/cycle is a structured cf branch in the body:
+! Both sets of bounds are carried in control(...):
+! CHECK: acc.loop combined(serial) private(%[[PRIVJ]], %[[PRIVI]] : !fir.ref<i32>, !fir.ref<i32>) control(%{{.*}} : i32, %{{.*}} : i32) = ({{.*}}) to ({{.*}}) step ({{.*}}) {
+! The if/cycle became an if/else, so the body's conditional is a fir.if:
! CHECK: arith.cmpi eq
-! CHECK: cf.cond_br
+! CHECK: fir.if
! CHECK: acc.yield
-! CHECK: }
+! End-of-line anchor: an `unstructured` attribute would follow `seq`.
+! CHECK: } inclusiveUpperbound({{.*}}) collapse([2]) collapseDeviceType([#acc.device_type<none>]) seq{{ *$}}
! `acc serial loop collapse(N)` with STOP in body: wrap-in-execute-region hides
! the unstructured if/stop and the three collapsed iterators lower as a single
@@ -334,9 +329,11 @@ subroutine test_unstructured_collapse_loop_only(a)
end subroutine
! CHECK-LABEL: func.func @_QPtest_unstructured_collapse_loop_only
-! Standalone acc.loop (no `combined(...)`):
-! CHECK: acc.loop private(%{{.*}}, %{{.*}} : !fir.ref<i32>, !fir.ref<i32>) {
-! CHECK: } collapse([2]) collapseDeviceType([#acc.device_type<none>]) independent unstructured
+! Standalone acc.loop (no `combined(...)`). The if/cycle is restructured into
+! an if/else, so the loop is structured and carries its bounds:
+! CHECK: acc.loop private(%{{.*}}, %{{.*}} : !fir.ref<i32>, !fir.ref<i32>) control(%{{.*}} : i32, %{{.*}} : i32) = ({{.*}}) to ({{.*}}) step ({{.*}}) {
+! End-of-line anchor: an `unstructured` attribute would follow `independent`.
+! CHECK: } inclusiveUpperbound({{.*}}) collapse([2]) collapseDeviceType([#acc.device_type<none>]) independent{{ *$}}
! Standalone `acc loop seq` with STOP: wrap-in-execute-region hides the
! if/stop and the DO lowers as structured acc.loop control(...) (no
diff --git a/flang/test/Lower/OpenMP/wsloop-unstructured-cycle.f90 b/flang/test/Lower/OpenMP/wsloop-unstructured-cycle.f90
index 60fe63b8d29afa..676266a71fa290 100644
--- a/flang/test/Lower/OpenMP/wsloop-unstructured-cycle.f90
+++ b/flang/test/Lower/OpenMP/wsloop-unstructured-cycle.f90
@@ -3,8 +3,12 @@
! A DO associated with an OpenMP loop directive is lowered by the directive's
! own code-gen. 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.
+! unstructured -- here a CYCLE nested inside an inner IF makes it so. The
+! body's blocks stay flat inside omp.loop_nest.
+!
+! The CYCLE must stay nested: one ending the outer IF body is rewritten into an
+! IF/ELSE and leaves the loop structured, and one in an ELSE branch makes the
+! construct wrappable, which trips the --implicit-check-not below.
!
! --implicit-check-not on the RUN line asserts that no wrapping takes place
! anywhere in the output.
@@ -18,8 +22,8 @@ subroutine repro_final(x, y, n)
!$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
+ if (y(1) > 0.0d0) cycle
+ y(1) = 0.0d0
end if
y(2) = 1.0d0
end do
@@ -34,11 +38,16 @@ end subroutine repro_final
! 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: cf.cond_br %{{[0-9]+}}, ^bb[[INNER:[0-9]+]], ^bb[[TAIL:[0-9]+]]
+! CHECK: ^bb[[INNER]]:
+! CHECK: arith.cmpf ogt
+! CHECK: cf.cond_br %{{[0-9]+}}, ^bb[[CYCLE:[0-9]+]], ^bb[[THEN:[0-9]+]]
! CHECK: ^bb[[CYCLE]]:
-! CHECK: hlfir.assign
! CHECK: cf.br ^bb[[EXIT:[0-9]+]]
-! CHECK: ^bb[[BODY]]:
+! CHECK: ^bb[[THEN]]:
+! CHECK: hlfir.assign
+! CHECK: cf.br ^bb[[TAIL]]
+! CHECK: ^bb[[TAIL]]:
! CHECK: hlfir.assign
! CHECK: cf.br ^bb[[EXIT]]
! CHECK: ^bb[[EXIT]]:
@@ -58,8 +67,8 @@ subroutine collapse_case(x, y, n)
do i = 1, n
do j = 1, n
if (x(i) > 0.0d0) then
+ if (y(1) > 0.0d0) cycle
y(1) = 0.0d0
- cycle
end if
y(2) = 1.0d0
end do
@@ -86,8 +95,8 @@ subroutine ordered_case(x, y, n)
do i = 1, n
do j = 1, n
if (x(i) > 0.0d0) then
+ if (y(1) > 0.0d0) cycle
y(1) = 0.0d0
- cycle
end if
y(2) = 1.0d0
end do
diff --git a/flang/test/Lower/if-cycle-to-if-else.f90 b/flang/test/Lower/if-cycle-to-if-else.f90
new file mode 100644
index 00000000000000..82d52d0db4b814
--- /dev/null
+++ b/flang/test/Lower/if-cycle-to-if-else.f90
@@ -0,0 +1,70 @@
+! RUN: %flang_fc1 -mmlir --wrap-unstructured-constructs-in-execute-region -fdebug-dump-pft %s 2>&1 | FileCheck %s
+
+! An IF body whose last statement is a CYCLE becomes an IF/ELSE, with the
+! statements after the construct moved into the ELSE branch. The condition is
+! not negated, the statements ahead of the CYCLE must still run when it holds.
+! A `!` suffix in this dump marks a construct as unstructured.
+
+subroutine trailing_cycle(n, v)
+ integer :: n, i, v(n)
+
+ ! CHECK-LABEL: Subroutine trailing_cycle
+ ! CHECK: <<DoConstruct>>
+ ! CHECK-NOT: DoConstruct!
+ ! CHECK: NonLabelDoStmt{{.*}}: do i = 1, n
+ ! CHECK: <<IfConstruct>>
+ ! CHECK-NOT: [negate]
+ ! CHECK: IfThenStmt{{.*}}: if(i == 1) then
+ ! CHECK: AssignmentStmt: v(i) = 0
+ ! CHECK: ElseStmt
+ ! CHECK: AssignmentStmt: v(i) = 1
+ ! CHECK: EndIfStmt
+ ! CHECK: <<End IfConstruct>>
+ ! CHECK: EndDoStmt
+ ! CHECK: <<End DoConstruct>>
+ ! CHECK-NOT: CycleStmt
+ do i = 1, n
+ if (i == 1) then
+ v(i) = 0
+ cycle
+ end if
+ v(i) = 1
+ end do
+end subroutine trailing_cycle
+
+! A CYCLE at the end of an ELSE branch is left alone: it is in the same
+! position within the construct, but the rewrite does not apply.
+
+subroutine cycle_in_else(n, v)
+ integer :: n, i, v(n)
+
+ ! CHECK-LABEL: Subroutine cycle_in_else
+ ! CHECK: CycleStmt
+ do i = 1, n
+ if (i == 1) then
+ v(i) = 0
+ else
+ v(i) = 1
+ cycle
+ end if
+ v(i) = 2
+ end do
+end subroutine cycle_in_else
+
+! A CYCLE naming an outer construct is left alone.
+
+subroutine named_outer_cycle(n, v)
+ integer :: n, i, j, v(n)
+
+ ! CHECK-LABEL: Subroutine named_outer_cycle
+ ! CHECK: CycleStmt
+ outer: do i = 1, n
+ inner: do j = 1, n
+ if (j == 1) then
+ v(i) = 0
+ cycle outer
+ end if
+ v(i) = 1
+ end do inner
+ end do outer
+end subroutine named_outer_cycle
``````````
</details>
https://github.com/llvm/llvm-project/pull/224718
More information about the flang-commits
mailing list