[flang-commits] [flang] [flang] Rewrite an IF body ending in CYCLE as an IF/ELSE (PR #224718)
Vijay Kandiah via flang-commits
flang-commits at lists.llvm.org
Mon Sep 21 20:47:22 PDT 2026
https://github.com/VijayKandiah updated https://github.com/llvm/llvm-project/pull/224718
>From 5acd6d77d3c0858040d16ffd50022dc7e367790b Mon Sep 17 00:00:00 2001
From: Vijay Kandiah <vkandiah at nvidia.com>
Date: Fri, 18 Sep 2026 12:43:27 -0700
Subject: [PATCH 1/4] [flang] Rewrite an IF body ending in CYCLE as an IF/ELSE
---
flang/lib/Lower/PFTBuilder.cpp | 113 +++++++++++++++++-
.../acc-unstructured-combined-construct.f90 | 2 +-
.../Todo/acc-unstructured-loop-construct.f90 | 2 +-
.../Lower/OpenACC/acc-loop-trailing-cycle.f90 | 29 +++++
flang/test/Lower/OpenACC/acc-unstructured.f90 | 33 +++--
.../OpenMP/wsloop-unstructured-cycle.f90 | 27 +++--
flang/test/Lower/if-cycle-to-if-else.f90 | 70 +++++++++++
7 files changed, 245 insertions(+), 31 deletions(-)
create mode 100644 flang/test/Lower/OpenACC/acc-loop-trailing-cycle.f90
create mode 100644 flang/test/Lower/if-cycle-to-if-else.f90
diff --git a/flang/lib/Lower/PFTBuilder.cpp b/flang/lib/Lower/PFTBuilder.cpp
index 82c16169b8e42..4f2d228f3ea2f 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 110940d9e5069..d070019b85804 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 f26e95d235d04..4a1af74f940a7 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 0000000000000..89f8d372bd213
--- /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 cbb27d74cc96c..b2a17915a43e3 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 60fe63b8d29af..676266a71fa29 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 0000000000000..82d52d0db4b81
--- /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
>From d2d43344ffe69037735d9b846d184679f792cb84 Mon Sep 17 00:00:00 2001
From: Vijay Kandiah <vkandiah at nvidia.com>
Date: Fri, 18 Sep 2026 12:53:32 -0700
Subject: [PATCH 2/4] [flang] Formatting fixes for PFT rewrite
---
flang/lib/Lower/PFTBuilder.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/flang/lib/Lower/PFTBuilder.cpp b/flang/lib/Lower/PFTBuilder.cpp
index 4f2d228f3ea2f..8237924abb3eb 100644
--- a/flang/lib/Lower/PFTBuilder.cpp
+++ b/flang/lib/Lower/PFTBuilder.cpp
@@ -869,7 +869,9 @@ class PFTBuilder {
std::string cycleName = getConstructName(*cycleStmt);
if (cycleName.empty() || cycleName == doName)
// This candidate will match doStmt's EndDoStmt.
- ifCandidateStack.push_back({it, {}, /*isCycleStmt=*/true,
+ ifCandidateStack.push_back({it,
+ {},
+ /*isCycleStmt=*/true,
/*hasLeadingStmts=*/true});
}
}
>From 991fbba29331dc7fd951c4fb3b8cd3d043d74858 Mon Sep 17 00:00:00 2001
From: Vijay Kandiah <vkandiah at nvidia.com>
Date: Fri, 18 Sep 2026 13:27:59 -0700
Subject: [PATCH 3/4] [flang][NFC] Test multiple IF/CYCLE constructs in one DO
---
flang/test/Lower/if-cycle-to-if-else.f90 | 92 ++++++++++++++++++++++++
1 file changed, 92 insertions(+)
diff --git a/flang/test/Lower/if-cycle-to-if-else.f90 b/flang/test/Lower/if-cycle-to-if-else.f90
index 82d52d0db4b81..425a9aaa51b38 100644
--- a/flang/test/Lower/if-cycle-to-if-else.f90
+++ b/flang/test/Lower/if-cycle-to-if-else.f90
@@ -51,6 +51,98 @@ subroutine cycle_in_else(n, v)
end do
end subroutine cycle_in_else
+! Two IF/CYCLEs in the same DO, both with statements ahead of the CYCLE, so
+! both are reshaped and the second nests inside the first's ELSE branch.
+
+subroutine two_cycles_both_leading(n, v)
+ integer :: n, i, v(n)
+
+ ! CHECK-LABEL: Subroutine two_cycles_both_leading
+ ! CHECK: <<DoConstruct>>
+ ! CHECK-NOT: DoConstruct!
+ ! CHECK: IfThenStmt{{.*}}: if(i == 1) then
+ ! CHECK: AssignmentStmt: v(i) = 0
+ ! CHECK: ElseStmt
+ ! CHECK: AssignmentStmt: v(i) = 1
+ ! CHECK: IfThenStmt{{.*}}: if(i == 2) then
+ ! CHECK: AssignmentStmt: v(i) = 3
+ ! CHECK: ElseStmt
+ ! CHECK: AssignmentStmt: v(i) = 2
+ ! CHECK: <<End DoConstruct>>
+ ! CHECK-NOT: CycleStmt
+ do i = 1, n
+ if (i == 1) then
+ v(i) = 0
+ cycle
+ end if
+ v(i) = 1
+ if (i == 2) then
+ v(i) = 3
+ cycle
+ end if
+ v(i) = 2
+ end do
+end subroutine two_cycles_both_leading
+
+! The reshaped construct is itself moved: the second IF is reshaped first, then
+! the first IF is negated and splices the result into the loop body.
+
+subroutine bare_then_leading(n, v)
+ integer :: n, i, v(n)
+
+ ! CHECK-LABEL: Subroutine bare_then_leading
+ ! CHECK: <<DoConstruct>>
+ ! CHECK-NOT: DoConstruct!
+ ! CHECK: IfThenStmt [negate]{{.*}}: if(i == 1) then
+ ! CHECK: AssignmentStmt: v(i) = 1
+ ! CHECK: IfThenStmt{{.*}}: if(i == 2) then
+ ! CHECK: AssignmentStmt: v(i) = 3
+ ! CHECK: ElseStmt
+ ! CHECK: AssignmentStmt: v(i) = 2
+ ! CHECK: <<End DoConstruct>>
+ ! CHECK-NOT: CycleStmt
+ do i = 1, n
+ if (i == 1) then
+ cycle
+ end if
+ v(i) = 1
+ if (i == 2) then
+ v(i) = 3
+ cycle
+ end if
+ v(i) = 2
+ end do
+end subroutine bare_then_leading
+
+! Nothing follows the second construct, so there is no ELSE branch to move and
+! its CYCLE is left alone. The DO stays structured: the branch goes to the
+! EndDoStmt and skips nothing.
+
+subroutine two_cycles_trailing(n, v)
+ integer :: n, i, v(n)
+
+ ! CHECK-LABEL: Subroutine two_cycles_trailing
+ ! CHECK: <<DoConstruct>>
+ ! CHECK-NOT: DoConstruct!
+ ! CHECK: IfThenStmt{{.*}}: if(i == 1) then
+ ! CHECK: AssignmentStmt: v(i) = 0
+ ! CHECK: ElseStmt
+ ! CHECK: AssignmentStmt: v(i) = 1
+ ! CHECK: IfThenStmt{{.*}}: if(i == 2) then
+ ! CHECK: CycleStmt
+ ! CHECK: <<End DoConstruct>>
+ do i = 1, n
+ if (i == 1) then
+ v(i) = 0
+ cycle
+ end if
+ v(i) = 1
+ if (i == 2) then
+ cycle
+ end if
+ end do
+end subroutine two_cycles_trailing
+
! A CYCLE naming an outer construct is left alone.
subroutine named_outer_cycle(n, v)
>From 6013f743a4adf9799b91f3c6994530b6d5c0b583 Mon Sep 17 00:00:00 2001
From: Vijay Kandiah <vkandiah at nvidia.com>
Date: Mon, 21 Sep 2026 20:46:48 -0700
Subject: [PATCH 4/4] [flang] Share the lexical predecessor search and skip
FORMAT statements
---
flang/include/flang/Lower/PFTBuilder.h | 7 ++
flang/lib/Lower/PFTBuilder.cpp | 85 +++++++++++++++---------
flang/test/Lower/if-cycle-to-if-else.f90 | 48 +++++++++++++
flang/test/Lower/trailing-cycle.f90 | 22 ++++++
4 files changed, 131 insertions(+), 31 deletions(-)
diff --git a/flang/include/flang/Lower/PFTBuilder.h b/flang/include/flang/Lower/PFTBuilder.h
index f495046c66a32..ae864bc87349c 100644
--- a/flang/include/flang/Lower/PFTBuilder.h
+++ b/flang/include/flang/Lower/PFTBuilder.h
@@ -278,6 +278,13 @@ struct Evaluation : EvaluationVariant {
}});
}
+ /// Return the predicate: "This evaluation is linked into the lexical
+ /// successor chain." FormatStmt and EntryStmt have no lexicalSuccessor.
+ constexpr bool isLexicallyLinked() const {
+ return isActionStmt() || isConstructStmt() || isEndStmt() ||
+ isExecutableDirective();
+ }
+
/// Return the predicate: "This is a non-initial, non-terminal construct
/// statement." For an IfConstruct, this is ElseIfStmt and ElseStmt.
constexpr bool isIntermediateConstructStmt() const {
diff --git a/flang/lib/Lower/PFTBuilder.cpp b/flang/lib/Lower/PFTBuilder.cpp
index 8237924abb3eb..ceb6245af4a65 100644
--- a/flang/lib/Lower/PFTBuilder.cpp
+++ b/flang/lib/Lower/PFTBuilder.cpp
@@ -633,8 +633,7 @@ class PFTBuilder {
lower::pft::FunctionLikeUnit *owningProcedure = eval.getOwningProcedure();
evaluationListStack.back()->emplace_back(std::move(eval));
lower::pft::Evaluation *p = &evaluationListStack.back()->back();
- if (p->isActionStmt() || p->isConstructStmt() || p->isEndStmt() ||
- p->isExecutableDirective()) {
+ if (p->isLexicallyLinked()) {
if (lastLexicalEvaluation) {
lastLexicalEvaluation->lexicalSuccessor = p;
p->printIndex = lastLexicalEvaluation->printIndex + 1;
@@ -678,6 +677,39 @@ class PFTBuilder {
evaluationListStack.pop_back();
}
+ /// Return the statement that opens an evaluation: the evaluation itself, or
+ /// its first nested statement if it is a construct.
+ static lower::pft::Evaluation *firstStmt(lower::pft::Evaluation *eval) {
+ return eval->isConstruct() ? &*eval->evaluationList->begin() : eval;
+ }
+
+ /// Return the last linked statement within an evaluation, descending
+ /// through nested evaluation lists, or nullptr if there is none.
+ static lower::pft::Evaluation *lastLinkedStmt(lower::pft::Evaluation *eval) {
+ if (eval->evaluationList)
+ for (lower::pft::Evaluation &nested :
+ llvm::reverse(*eval->evaluationList))
+ if (lower::pft::Evaluation *stmt = lastLinkedStmt(&nested))
+ return stmt;
+ return eval->isLexicallyLinked() ? eval : nullptr;
+ }
+
+ /// Return the evaluation that falls through to the evaluation at \p it.
+ /// Unlinked statements are skipped, and a preceding construct is descended
+ /// into to reach the last statement of its body.
+ static lower::pft::Evaluation *
+ fallthroughPredecessor(lower::pft::EvaluationList &evaluationList,
+ lower::pft::EvaluationList::iterator it) {
+ lower::pft::Evaluation *predecessor = nullptr;
+ for (lower::pft::EvaluationList::iterator searchIt = it;
+ !predecessor && searchIt != evaluationList.begin();)
+ predecessor = lastLinkedStmt(&*--searchIt);
+ assert(predecessor && "no fallthrough predecessor");
+ assert(predecessor->lexicalSuccessor == firstStmt(&*it) &&
+ "expected fallthrough to the next evaluation");
+ return predecessor;
+ }
+
/// Delete a CycleStmt that is the last statement of the body of its own
/// DoConstruct, where it is a no-op. The pre-branch-analysis code:
///
@@ -720,13 +752,9 @@ class PFTBuilder {
std::string cycleName = getConstructName(*cycleStmt);
if (!cycleName.empty() && cycleName != getConstructName(*doStmt))
return; // cycle for an outer construct
- // Relink the lexical predecessor of the CycleStmt to the EndDoStmt. That
- // predecessor is the last statement reachable from the preceding
- // evaluation, so descend through nested evaluation lists to find it.
- lower::pft::Evaluation *predecessor = &*std::prev(cycleStmtIt);
- while (predecessor->evaluationList && !predecessor->evaluationList->empty())
- predecessor = &predecessor->evaluationList->back();
- assert(predecessor->lexicalSuccessor == &*cycleStmtIt);
+ // Relink the lexical predecessor of the CycleStmt to the EndDoStmt.
+ lower::pft::Evaluation *predecessor =
+ fallthroughPredecessor(evaluationList, cycleStmtIt);
predecessor->lexicalSuccessor = cycleStmtIt->lexicalSuccessor;
evaluationList.erase(cycleStmtIt);
}
@@ -793,9 +821,6 @@ class PFTBuilder {
ifCandidateStack.clear();
continue;
}
- auto firstStmt = [](lower::pft::Evaluation *e) {
- return e->isConstruct() ? &*e->evaluationList->begin() : e;
- };
const Fortran::lower::pft::Evaluation &targetEval = *firstStmt(&eval);
bool targetEvalIsEndDoStmt = targetEval.isA<parser::EndDoStmt>();
auto branchTargetMatch = [&]() {
@@ -816,7 +841,7 @@ class PFTBuilder {
if (successorIt != it && ifCandidateStack.back().hasLeadingStmts) {
rewriteIfCycleAsIfElse(evaluationList,
*ifConstructIt->evaluationList,
- ifConstructIt, successorIt, it, firstStmt);
+ ifConstructIt, successorIt, it);
} else if (successorIt != it) {
Fortran::lower::pft::EvaluationList &ifBodyList =
*ifConstructIt->evaluationList;
@@ -908,30 +933,31 @@ class PFTBuilder {
/// 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 it) {
lower::pft::EvaluationList::iterator branchStmtIt =
std::prev(std::prev(ifBodyList.end()));
assert(branchStmtIt->isA<parser::CycleStmt>() &&
"expected cycle statement");
+ lower::pft::EvaluationList::iterator endIfStmtIt =
+ std::prev(ifBodyList.end());
+
+ // The ELSE branch starts where the EndIfStmt currently falls through. If
+ // that is the branch target, nothing linked lies between, so nothing
+ // moves.
+ lower::pft::Evaluation *elseBranchStart = endIfStmtIt->lexicalSuccessor;
+ if (elseBranchStart == firstStmt(&*it))
+ return;
- // 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");
+ // The THEN branch tail, which currently falls through to the CycleStmt.
+ lower::pft::Evaluation *thenTail =
+ fallthroughPredecessor(ifBodyList, branchStmtIt);
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.
@@ -944,14 +970,11 @@ class PFTBuilder {
// 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");
+ lower::pft::Evaluation *movedTail =
+ fallthroughPredecessor(evaluationList, it);
thenTail->lexicalSuccessor = &*elseStmtIt;
- elseStmtIt->lexicalSuccessor = firstStmt(&*successorIt);
+ elseStmtIt->lexicalSuccessor = elseBranchStart;
movedTail->lexicalSuccessor = &*endIfStmtIt;
endIfStmtIt->lexicalSuccessor = firstStmt(&*it);
diff --git a/flang/test/Lower/if-cycle-to-if-else.f90 b/flang/test/Lower/if-cycle-to-if-else.f90
index 425a9aaa51b38..3befeac89ceb9 100644
--- a/flang/test/Lower/if-cycle-to-if-else.f90
+++ b/flang/test/Lower/if-cycle-to-if-else.f90
@@ -160,3 +160,51 @@ subroutine named_outer_cycle(n, v)
end do inner
end do outer
end subroutine named_outer_cycle
+
+! A FORMAT statement is not in the lexical chain, so the predecessor of the
+! CYCLE is the assignment ahead of it, and that is what falls through to the
+! synthesized ElseStmt.
+
+subroutine format_in_then(n, v)
+ integer :: n, i, v(n)
+
+ ! CHECK-LABEL: Subroutine format_in_then
+ ! CHECK: <<DoConstruct>>
+ ! CHECK-NOT: DoConstruct!
+ ! CHECK: IfThenStmt
+ ! CHECK-NOT: [negate]
+ ! CHECK: AssignmentStmt: v(i) = 7
+ ! CHECK: FormatStmt
+ ! CHECK: ElseStmt
+ ! CHECK: AssignmentStmt: v(i) = 2
+ ! CHECK: EndIfStmt
+ ! CHECK: <<End IfConstruct>>
+ ! CHECK: EndDoStmt
+ ! CHECK: <<End DoConstruct>>
+ ! CHECK-NOT: CycleStmt
+ do i = 1, n
+ if (v(i) == 1) then
+ v(i) = 7
+100 format(I5)
+ cycle
+ end if
+ v(i) = 2
+ end do
+end subroutine format_in_then
+
+! Everything between the construct and the EndDoStmt is a FORMAT, so there is
+! no statement for an ELSE branch and the CYCLE is left alone.
+
+subroutine format_only_after_if(n, v)
+ integer :: n, i, v(n)
+
+ ! CHECK-LABEL: Subroutine format_only_after_if
+ ! CHECK: CycleStmt
+ do i = 1, n
+ if (v(i) == 1) then
+ v(i) = 7
+ cycle
+ end if
+200 format(I5)
+ end do
+end subroutine format_only_after_if
diff --git a/flang/test/Lower/trailing-cycle.f90 b/flang/test/Lower/trailing-cycle.f90
index ed4ca7f17f0d5..2b991bf7c973a 100644
--- a/flang/test/Lower/trailing-cycle.f90
+++ b/flang/test/Lower/trailing-cycle.f90
@@ -139,3 +139,25 @@ subroutine trailing_cycle(a, n)
cycle
end do
end subroutine trailing_cycle
+
+! A FORMAT statement is not in the lexical chain, so the predecessor of the
+! CYCLE is the assignment ahead of it, and that is what gets relinked.
+
+! CHECK: Subroutine format_before_cycle
+subroutine format_before_cycle(a, n)
+ integer :: n, i
+ real :: a(n)
+
+ ! CHECK: <<DoConstruct>> -> 5
+ ! CHECK: 1 NonLabelDoStmt -> 4: do i = 1, n
+ ! CHECK: 2 ^AssignmentStmt: a(i) = 1.0
+ ! CHECK: FormatStmt
+ ! CHECK: 4 EndDoStmt -> 1: end do
+ ! CHECK: <<End DoConstruct>>
+ ! CHECK-NOT: CycleStmt
+ do i = 1, n
+ a(i) = 1.0
+100 format(I5)
+ cycle
+ end do
+end subroutine format_before_cycle
More information about the flang-commits
mailing list