[flang-commits] [flang] [flang] Detect loops whose branching is confined to their body (PR #225757)

Kareem Ergawy via flang-commits flang-commits at lists.llvm.org
Tue Sep 29 04:49:24 PDT 2026


https://github.com/ergawy updated https://github.com/llvm/llvm-project/pull/225757

>From cacf83e9dfce8fd7d18a050804bd510acc134240 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at gmail.com>
Date: Mon, 21 Sep 2026 06:16:09 -0700
Subject: [PATCH 1/5] [flang] Detect loops whose branching is confined to their
 body

A DO loop is classified as either structured or unstructured, and a single
raw branch anywhere in its body forces the loop -- and every construct
enclosing it -- onto the unstructured path.

That is stronger than necessary. A loop keeps its structured control flow
as long as its branching neither leaves its body nor enters it from
outside. Classify such a loop separately from a fully unstructured one.

This only classifies: lowering is unchanged. PFT dumps mark the new
classification with '~', which is what the tests key on.
---
 flang/include/flang/Lower/PFTBuilder.h        |  57 ++++-
 flang/lib/Lower/PFTBuilder.cpp                | 214 ++++++++++++++++--
 .../pre-fir-tree-unstructured-internals.f90   | 137 +++++++++++
 flang/test/Lower/trailing-cycle.f90           |  15 +-
 4 files changed, 388 insertions(+), 35 deletions(-)
 create mode 100644 flang/test/Lower/pre-fir-tree-unstructured-internals.f90

diff --git a/flang/include/flang/Lower/PFTBuilder.h b/flang/include/flang/Lower/PFTBuilder.h
index 7e748ae554a20..6bf9dcb76f9e7 100644
--- a/flang/include/flang/Lower/PFTBuilder.h
+++ b/flang/include/flang/Lower/PFTBuilder.h
@@ -28,6 +28,7 @@
 #include "flang/Semantics/symbol.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
+#include <algorithm>
 
 namespace Fortran::lower::pft {
 
@@ -322,6 +323,50 @@ struct Evaluation : EvaluationVariant {
   /// Return the FunctionLikeUnit containing this evaluation (or nullptr).
   FunctionLikeUnit *getOwningProcedure() const;
 
+  /// How this evaluation's control flow is lowered. Ordered by how much it
+  /// constrains lowering so that classification can only strengthen; see
+  /// markControlFlow.
+  enum class ControlFlow {
+    /// Lowered structurally
+    Structured,
+    /// Lowered structurally, but the body holds unstructured control flow
+    /// confined to it. Lowering folds that body -- not the construct, and not
+    /// the loop control -- into a parent region, so the structured op's
+    /// single-block region stays well formed.
+    ///
+    /// For now, this only applies to DO constructs.
+    StructuredWithUnstructuredInternals,
+    /// Lowered as unstructured blocks.
+    Unstructured,
+  };
+
+  /// Strengthen the classification to \p kind; it never weakens. This is what
+  /// makes the analysis order-independent: a construct marked Unstructured by
+  /// any one child stays Unstructured whatever its siblings contribute.
+  void markControlFlow(ControlFlow kind) {
+    controlFlow = std::max(controlFlow, kind);
+  }
+
+  void markUnstructured() { markControlFlow(ControlFlow::Unstructured); }
+
+  /// Lower the classification to \p kind. Only for an analysis that has proven
+  /// a stronger classification unnecessary; every other caller wants
+  /// markControlFlow, which never weakens.
+  void weakenControlFlow(ControlFlow kind) {
+    controlFlow = std::min(controlFlow, kind);
+  }
+
+  /// True when control flow is not fully structured, category (c) included.
+  /// Existing consumers ask this to decide whether raw blocks are needed, and
+  /// a category (c) construct still needs them until its body is wrapped, so
+  /// it must answer true here. Use hasUnstructuredInternals() to single out
+  /// category (c) itself.
+  bool isUnstructured() const { return controlFlow != ControlFlow::Structured; }
+
+  bool hasUnstructuredInternals() const {
+    return controlFlow == ControlFlow::StructuredWithUnstructuredInternals;
+  }
+
   bool lowerAsStructured() const;
   bool lowerAsUnstructured() const;
   bool forceAsUnstructured() const;
@@ -339,11 +384,11 @@ struct Evaluation : EvaluationVariant {
   // from anywhere within the construct.
   //
   // An unstructured construct is one that contains some form of goto. This
-  // is indicated by the isUnstructured member flag, which may be set on a
-  // statement and propagated to enclosing constructs. This distinction allows
-  // a structured IF or DO statement to be materialized with custom structured
-  // FIR operations. An unstructured statement is materialized as mlir
-  // operation sequences that include explicit branches.
+  // is indicated by the controlFlow member, which may be set on a statement and
+  // propagated to enclosing constructs. This distinction allows a structured IF
+  // or DO statement to be materialized with custom structured FIR operations.
+  // An unstructured statement is materialized as mlir operation sequences that
+  // include explicit branches.
   //
   // The block member is set for statements that begin a new block. This
   // block is the target of any branch to the statement. Statements may have
@@ -374,7 +419,7 @@ struct Evaluation : EvaluationVariant {
   llvm::SmallVector<Evaluation *, 0> extraControlSuccessors;
   Evaluation *constructExit{nullptr};    // set for constructs
   bool isNewBlock{false};                // evaluation begins a new basic block
-  bool isUnstructured{false};  // evaluation has unstructured control flow
+  ControlFlow controlFlow{ControlFlow::Structured};
   bool negateCondition{false}; // If[Then]Stmt condition must be negated
   bool activeConstruct{false}; // temporarily set for some constructs
   // The enclosing evaluation-list traversal should skip this evaluation once
diff --git a/flang/lib/Lower/PFTBuilder.cpp b/flang/lib/Lower/PFTBuilder.cpp
index e6a4d11904679..bc66c80aa68fe 100644
--- a/flang/lib/Lower/PFTBuilder.cpp
+++ b/flang/lib/Lower/PFTBuilder.cpp
@@ -34,6 +34,9 @@ llvm::cl::opt<bool> wrapUnstructuredConstructsInExecuteRegion(
 
 using namespace Fortran;
 
+static void detectStructuredWithUnstructuredInternals(
+    Fortran::lower::pft::FunctionLikeUnit &unit);
+
 namespace {
 static llvm::cl::opt<bool> lowerDoWhileToSCFWhile(
     "lower-do-while-to-scf-while", llvm::cl::init(false),
@@ -525,11 +528,16 @@ class PFTBuilder {
   }
 
   void exitFunction() {
+    lower::pft::FunctionLikeUnit *exitingUnit = currentFunctionUnit;
     currentFunctionUnit = nullptr; // Clear when exiting function
     rewriteIfGotos();
     endFunctionBody();
     analyzeBranches(nullptr, *evaluationListStack.back()); // add branch links
 
+    // Branch analysis is complete, so the incoming-branch map is too.
+    if (exitingUnit)
+      detectStructuredWithUnstructuredInternals(*exitingUnit);
+
     processEntryPoints();
     containsStmtStack.pop_back();
     popEvaluationList();
@@ -862,7 +870,7 @@ class PFTBuilder {
       if (const auto *expr = std::get_if<parser::Expr>(&format.u)) {
         if (semantics::ExprHasTypeCategory(*semantics::GetExpr(*expr),
                                            common::TypeCategory::Integer))
-          eval.isUnstructured = true;
+          eval.markUnstructured();
       }
     };
     auto analyzeSpecs{[&](const auto &specList) {
@@ -916,7 +924,7 @@ class PFTBuilder {
   /// Mark the target of a branch as a new block.
   void markBranchTarget(lower::pft::Evaluation &sourceEvaluation,
                         lower::pft::Evaluation &targetEvaluation) {
-    sourceEvaluation.isUnstructured = true;
+    sourceEvaluation.markUnstructured();
     if (!sourceEvaluation.controlSuccessor)
       sourceEvaluation.controlSuccessor = &targetEvaluation;
     else if (sourceEvaluation.controlSuccessor != &targetEvaluation &&
@@ -942,11 +950,11 @@ class PFTBuilder {
       if (sourceConstruct != targetConstruct) // branch into a construct body
         for (lower::pft::Evaluation *eval = &targetEvaluation; eval;
              eval = eval->parentConstruct) {
-          eval->isUnstructured = true;
+          eval->markUnstructured();
           // If the branch is a backward branch into an already analyzed
           // DO or IF construct, mark the construct exit as a new block.
-          // For a forward branch, the isUnstructured flag will cause this
-          // to be done when the construct is analyzed.
+          // For a forward branch, the Unstructured classification will cause
+          // this to be done when the construct is analyzed.
           if (eval->constructExit && (eval->isA<parser::DoConstruct>() ||
                                       eval->isA<parser::IfConstruct>()))
             eval->constructExit->isNewBlock = true;
@@ -1051,7 +1059,7 @@ class PFTBuilder {
             markBranchTarget(eval, *construct->constructExit);
           },
           [&](const parser::FailImageStmt &) {
-            eval.isUnstructured = true;
+            eval.markUnstructured();
             if (eval.lexicalSuccessor->lexicalSuccessor)
               markSuccessorAsNewBlock(eval);
           },
@@ -1061,12 +1069,12 @@ class PFTBuilder {
             lastConstructStmtEvaluation = &eval;
           },
           [&](const parser::ReturnStmt &) {
-            eval.isUnstructured = true;
+            eval.markUnstructured();
             if (eval.lexicalSuccessor->lexicalSuccessor)
               markSuccessorAsNewBlock(eval);
           },
           [&](const parser::StopStmt &) {
-            eval.isUnstructured = true;
+            eval.markUnstructured();
             if (eval.lexicalSuccessor->lexicalSuccessor)
               markSuccessorAsNewBlock(eval);
           },
@@ -1094,7 +1102,7 @@ class PFTBuilder {
               target->isNewBlock = true;
               for (lower::pft::Evaluation *parent = target->parentConstruct;
                    parent; parent = parent->parentConstruct) {
-                parent->isUnstructured = true;
+                parent->markUnstructured();
                 // The exit of an enclosing DO or IF construct is a new block.
                 if (parent->constructExit &&
                     (parent->isA<parser::DoConstruct>() ||
@@ -1140,7 +1148,7 @@ class PFTBuilder {
                 for (auto label : iter->second)
                   markIfBranchTarget(label);
             }
-            eval.isUnstructured = true;
+            eval.markUnstructured();
             markSuccessorAsNewBlock(eval);
           },
 
@@ -1181,7 +1189,7 @@ class PFTBuilder {
             const auto &loopControl =
                 std::get<std::optional<parser::LoopControl>>(s.t);
             if (!loopControl.has_value()) {
-              eval.isUnstructured = true; // infinite loop
+              eval.markUnstructured(); // infinite loop
               return;
             }
             eval.nonNopSuccessor().isNewBlock = true;
@@ -1190,13 +1198,13 @@ class PFTBuilder {
                     std::get_if<parser::LoopControl::Bounds>(&loopControl->u)) {
               if (bounds->Name().thing.symbol->GetType()->IsNumeric(
                       common::TypeCategory::Real))
-                eval.isUnstructured = true; // real-valued loop control
+                eval.markUnstructured(); // real-valued loop control
             } else if (std::get_if<parser::ScalarLogicalExpr>(
                            &loopControl->u)) {
               // Leave DO WHILE structured when -lower-do-while-to-scf-while is
               // enabled; branch analysis will mark unstructured cases.
               if (!lowerDoWhileToSCFWhile)
-                eval.isUnstructured = true; // while loop
+                eval.markUnstructured(); // while loop
             }
           },
           [&](const parser::EndDoStmt &) {
@@ -1277,7 +1285,7 @@ class PFTBuilder {
           },
           [&](const parser::CaseConstruct &) {
             eval.constructExit = &eval.evaluationList->back();
-            eval.isUnstructured = true;
+            eval.markUnstructured();
           },
           [&](const parser::ChangeTeamConstruct &) {
             eval.constructExit = &eval.evaluationList->back();
@@ -1290,11 +1298,11 @@ class PFTBuilder {
           [&](const parser::IfConstruct &) { setConstructExit(eval); },
           [&](const parser::SelectRankConstruct &) {
             eval.constructExit = &eval.evaluationList->back();
-            eval.isUnstructured = true;
+            eval.markUnstructured();
           },
           [&](const parser::SelectTypeConstruct &) {
             eval.constructExit = &eval.evaluationList->back();
-            eval.isUnstructured = true;
+            eval.markUnstructured();
           },
           [&](const parser::WhereConstruct &) { setConstructExit(eval); },
 
@@ -1315,12 +1323,12 @@ class PFTBuilder {
       if (eval.evaluationList)
         analyzeBranches(&eval, *eval.evaluationList);
 
-      // Propagate isUnstructured flag to enclosing construct -- unless the
-      // wrap pass will fold this construct into a self-contained
+      // Propagate the Unstructured classification to the enclosing construct --
+      // unless the wrap pass will fold this construct into a self-contained
       // scf.execute_region, in which case the parent sees only a single op.
-      if (parentConstruct && eval.isUnstructured &&
+      if (parentConstruct && eval.isUnstructured() &&
           !lower::pft::isWrappableConstruct(eval, semanticsContext))
-        parentConstruct->isUnstructured = true;
+        parentConstruct->markUnstructured();
 
       // The successor of a branch starts a new block.
       if (eval.controlSuccessor && eval.isActionStmt() &&
@@ -1472,7 +1480,11 @@ class PFTDumper {
                       const std::string &indentString, int indent = 1) {
     llvm::StringRef name = evaluationName(eval);
     llvm::StringRef newBlock = eval.isNewBlock ? "^" : "";
-    llvm::StringRef bang = eval.isUnstructured ? "!" : "";
+    // "!" marks an unstructured evaluation. "~" marks one that is structured
+    // on the outside but has unstructured internals.
+    llvm::StringRef bang = eval.hasUnstructuredInternals()
+                               ? "~"
+                               : (eval.isUnstructured() ? "!" : "");
     outputStream << indentString;
     if (eval.printIndex)
       outputStream << eval.printIndex << ' ';
@@ -1720,7 +1732,7 @@ bool Fortran::lower::pft::Evaluation::lowerAsStructured() const {
 }
 
 bool Fortran::lower::pft::Evaluation::lowerAsUnstructured() const {
-  return isUnstructured || clDisableStructuredFir;
+  return isUnstructured() || clDisableStructuredFir;
 }
 
 bool Fortran::lower::pft::Evaluation::forceAsUnstructured() const {
@@ -2750,13 +2762,169 @@ static bool isOmpLoopBody(const Fortran::lower::pft::Evaluation &eval,
   return isAssociatedLoop(chain, loop->GetNestedLoop(), n);
 }
 
+/// The evaluations forming a loop's body: everything between the loop control
+/// statements, which bracket it and are lowered outside any body wrap.
+static llvm::iterator_range<Fortran::lower::pft::EvaluationList::const_iterator>
+loopBodyRange(const Fortran::lower::pft::Evaluation &loop) {
+  const auto &list = *loop.evaluationList;
+  return llvm::make_range(std::next(list.begin()), std::prev(list.end()));
+}
+
+/// True when \p eval lies in \p loop's body rather than in its loop control.
+static bool isInLoopBody(const Fortran::lower::pft::Evaluation *eval,
+                         const Fortran::lower::pft::Evaluation &loop) {
+  if (!eval || !loop.evaluationList || loop.evaluationList->empty())
+    return false;
+  const Fortran::lower::pft::Evaluation *first = &loop.evaluationList->front();
+  const Fortran::lower::pft::Evaluation *last = &loop.evaluationList->back();
+  for (const Fortran::lower::pft::Evaluation *p = eval; p;
+       p = p->parentConstruct)
+    if (p->parentConstruct == &loop)
+      return p != first && p != last;
+  return false;
+}
+
+/// A loop that can be lowered structurally even though its body holds
+/// unstructured control flow, because that control flow is confined to the body
+/// and can be folded into an SESE region.
+///
+/// The loop qualifies when:
+///   1. every branch leaving its body lands back inside that body, and
+///   2. every branch into its body comes from inside that body,
+/// and the body holds nothing a region cannot accommodate: an infinite DO
+/// never reaches the region's yield so RegionDCE would drop it, a ReturnStmt
+/// builds the function's final block in the current region, and a listless
+/// assigned GO TO has targets that cannot be enumerated -- so condition 1
+/// cannot be decided at all rather than merely failing.
+///
+/// A CYCLE is not an escape: its target is the EndDoStmt, which is where the
+/// wrap's yield sits, so it lands on the boundary. An EXIT targets the
+/// construct exit, beyond the loop entirely, and does escape.
+static bool isStructurableWithUnstructuredInternals(
+    const Fortran::lower::pft::Evaluation &loop,
+    const Fortran::lower::pft::FunctionLikeUnit &unit) {
+
+  if (!loop.isUnstructured() || !loop.evaluationList ||
+      loop.evaluationList->size() < 3)
+    return false;
+
+  // Only an increment loop keeps all of its control outside the body. A DO
+  // WHILE or an infinite DO lowers through a header block and a back edge, and
+  // a do concurrent has no plain bounds triple either, so in each case the
+  // loop's own control flow runs through the body a wrap would cover.
+  const auto *doConstruct = loop.getIf<parser::DoConstruct>();
+  if (!doConstruct)
+    return false;
+
+  const auto &loopControl = doConstruct->GetLoopControl();
+  if (!loopControl)
+    return false;
+
+  const auto *bounds =
+      std::get_if<parser::LoopControl::Bounds>(&loopControl->u);
+  if (!bounds)
+    return false;
+
+  // A REAL control variable does not lower to fir.do_loop, whose induction
+  // variable must be a signless integer or index, so such a loop is lowered as
+  // unstructured whatever its body looks like.
+  const semantics::Symbol *ctrlVar = bounds->Name().thing.symbol;
+  if (!ctrlVar)
+    return false;
+
+  const semantics::DeclTypeSpec *ctrlType = ctrlVar->GetType();
+  if (!ctrlType || ctrlType->category() != semantics::DeclTypeSpec::Numeric ||
+      ctrlType->numericTypeSpec().category() != common::TypeCategory::Integer)
+    return false;
+
+  const Fortran::lower::pft::Evaluation *endDoStmt =
+      &loop.evaluationList->back();
+
+  auto isInfiniteDo = [](const parser::DoConstruct *d) {
+    return d && !d->GetLoopControl().has_value();
+  };
+
+  auto targetEscapes = [&](const Fortran::lower::pft::Evaluation *target) {
+    return target != endDoStmt && !isInLoopBody(target, loop);
+  };
+
+  std::function<bool(const Fortran::lower::pft::Evaluation &)> check =
+      [&](const Fortran::lower::pft::Evaluation &e) -> bool {
+    if (e.isA<parser::ReturnStmt>() ||
+        isInfiniteDo(e.getIf<parser::DoConstruct>()))
+      return false;
+
+    if (const auto *g = e.getIf<parser::AssignedGotoStmt>())
+      if (std::get<std::list<parser::Label>>(g->t).empty())
+        return false;
+
+    // Condition 1: nothing leaves the body, CYCLE excepted.
+    if (e.controlSuccessor && targetEscapes(e.controlSuccessor))
+      return false;
+
+    for (const Fortran::lower::pft::Evaluation *extra :
+         e.extraControlSuccessors)
+      if (targetEscapes(extra))
+        return false;
+
+    // Condition 2: nothing enters the body from outside it. This is the
+    // lookup the incoming-branch map exists for.
+    auto it = unit.incomingBranches.find(&e);
+    if (it != unit.incomingBranches.end())
+      for (const Fortran::lower::pft::Evaluation *src : it->second)
+        if (!isInLoopBody(src, loop))
+          return false;
+
+    if (e.evaluationList)
+      for (const Fortran::lower::pft::Evaluation &nested : *e.evaluationList)
+        if (!check(nested))
+          return false;
+
+    return true;
+  };
+
+  for (const Fortran::lower::pft::Evaluation &e : loopBodyRange(loop))
+    if (!check(e))
+      return false;
+
+  return true;
+}
+
+/// Reclassify every qualifying loop in \p unit.
+///
+/// Runs after branch analysis, when the incoming-branch map is complete;
+/// during analysis a branch later in the function would not yet be recorded
+/// and condition 2 would read a partial map.
+///
+/// Ancestors are deliberately left alone. A loop reclassified here lowers to a
+/// structured op, and a structured op is legal inside an unstructured parent,
+/// so leaving the parent Unstructured is conservative but correct.
+static void detectStructuredWithUnstructuredInternals(
+    Fortran::lower::pft::FunctionLikeUnit &unit) {
+  std::function<void(Fortran::lower::pft::EvaluationList &)> visit =
+      [&](Fortran::lower::pft::EvaluationList &list) {
+        for (Fortran::lower::pft::Evaluation &e : list) {
+          if (e.evaluationList)
+            visit(*e.evaluationList);
+
+          if (e.isA<parser::DoConstruct>() &&
+              isStructurableWithUnstructuredInternals(e, unit))
+            // The one place the classification weakens: detection has proven
+            // Unstructured unnecessary.
+            e.weakenControlFlow(Fortran::lower::pft::Evaluation::ControlFlow::
+                                    StructuredWithUnstructuredInternals);
+        }
+      };
+  visit(unit.evaluationList);
+}
+
 bool Fortran::lower::pft::isWrappableConstruct(
     const Fortran::lower::pft::Evaluation &eval,
     const Fortran::semantics::SemanticsContext &semaCtx) {
   if (!wrapUnstructuredConstructsInExecuteRegion)
     return false;
 
-  if (!eval.isUnstructured)
+  if (!eval.isUnstructured())
     return false;
 
   if (!(eval.isA<Fortran::parser::DoConstruct>() ||
diff --git a/flang/test/Lower/pre-fir-tree-unstructured-internals.f90 b/flang/test/Lower/pre-fir-tree-unstructured-internals.f90
new file mode 100644
index 0000000000000..30236e51bb40b
--- /dev/null
+++ b/flang/test/Lower/pre-fir-tree-unstructured-internals.f90
@@ -0,0 +1,137 @@
+! RUN: %flang_fc1 -fdebug-dump-pft %s 2>&1 | FileCheck %s
+
+! Detection of loops whose control flow is structured on the outside but has
+! raw branching confined to the body. Such a loop is marked '~' in the dump, as
+! opposed to '!' for a fully unstructured one and no marker for a structured
+! one.
+
+! Two forward GOTOs that both land inside the body. Nothing enters the body
+! from outside and nothing leaves it, so the loop qualifies even though its
+! internals are branch-based.
+subroutine internal_gotos(a, n)
+  real :: a(n)
+  ! CHECK:   <<DoConstruct~>> -> 10
+  ! CHECK:     6 GotoStmt! -> 8: goto 20
+  ! CHECK:     8 ^AssignmentStmt <- 6: 20 a(i) = a(i) + 1.0
+  ! CHECK:   <<End DoConstruct~>>
+  do i = 1, n
+    if (a(i) > 0.0) goto 10
+    a(i) = 1.0
+    goto 20
+10  a(i) = 2.0
+20  a(i) = a(i) + 1.0
+  end do
+end subroutine
+
+! A CYCLE targets the EndDoStmt, which is the boundary between the body and the
+! loop control, so it does not count as escaping the body.
+subroutine cycle_in_if_block(a, n)
+  real :: a(n)
+  ! CHECK:   <<DoConstruct~>> -> 8
+  ! CHECK:     [[CYC:[0-9]+]] CycleStmt! -> [[END:[0-9]+]]: cycle
+  ! CHECK:     [[END]] ^EndDoStmt -> 1 <- [[CYC]]: end do
+  ! CHECK:   <<End DoConstruct~>>
+  do i = 1, n
+    if (a(i) > 0.0) then
+      a(i) = 1.0
+      cycle
+    end if
+    a(i) = 2.0
+  end do
+end subroutine
+
+! The GOTO leaves the loop entirely, so the branch graph is not contained.
+subroutine escaping_goto(a, n)
+  real :: a(n)
+  ! CHECK:   <<DoConstruct!>> -> 7
+  ! CHECK:   <<End DoConstruct!>>
+  do i = 1, n
+    if (a(i) > 0.0) goto 20
+    a(i) = 2.0
+  end do
+20 continue
+end subroutine
+
+! A RETURN escapes the loop and the procedure both.
+subroutine body_has_return(a, n)
+  real :: a(n)
+  ! CHECK:   <<DoConstruct!>> -> 7
+  ! CHECK:   <<End DoConstruct!>>
+  do i = 1, n
+    if (a(i) > 0.0) return
+    a(i) = 3.0
+  end do
+end subroutine
+
+! The branch originates outside the loop and lands inside its body, so the body
+! has an external entry point and cannot be wrapped.
+subroutine incoming_from_outside(a, n)
+  real :: a(n)
+  ! CHECK:   <<DoConstruct!>> -> 8
+  ! CHECK:   <<End DoConstruct!>>
+  if (n < 0) goto 30
+  do i = 1, n
+    a(i) = 4.0
+30  continue
+  end do
+end subroutine
+
+! An inner infinite DO has no structured loop control to preserve.
+subroutine infinite_inner(a, n)
+  real :: a(n)
+  ! CHECK:   <<DoConstruct!>> -> 9
+  ! CHECK:     <<DoConstruct!>> -> 8
+  ! CHECK:     <<End DoConstruct!>>
+  ! CHECK:   <<End DoConstruct!>>
+  do i = 1, n
+    do
+      a(i) = 1.0
+      if (a(i) > 0.0) exit
+    end do
+  end do
+end subroutine
+
+! The shape that motivates this work: a forward GOTO raised inside a nested
+! IF, jumping over a whole inner DO construct and landing on the last statement
+! of the outer loop body. Both endpoints are inside the body, so the outer loop
+! is category (c) even though the branch crosses construct boundaries. Note the
+! two inner DO constructs stay structured -- only the outer loop carries the
+! branching.
+subroutine goto_over_inner_loop(qfx, hfx, a, its, ite, jts, jte, force, flux)
+  real :: qfx(ite,jte), hfx(ite,jte), a(ite,jte)
+  logical :: force
+  integer :: flux
+  ! CHECK:   <<DoConstruct~>> -> 18
+  ! CHECK:     <<DoConstruct>> -> 6
+  ! CHECK:     <<End DoConstruct>>
+  ! CHECK:       [[GOTO:[0-9]+]] ^GotoStmt! -> [[TGT:[0-9]+]]: goto 350
+  ! CHECK:     <<DoConstruct>> -> [[TGT]]
+  ! CHECK:     <<End DoConstruct>>
+  ! CHECK:     [[TGT]] ^ContinueStmt <- [[GOTO]]: 350 continue
+  ! CHECK:     17 EndDoStmt -> 1: enddo
+  ! CHECK:   <<End DoConstruct~>>
+  do j = jts, jte
+    do 330 i = its, ite
+      a(i,j) = a(i,j) + 1.0
+330 continue
+335 continue
+    if (force) then
+      if (flux .eq. 1) goto 350
+    endif
+    do i = its, ite
+      qfx(i,j) = 0.
+      hfx(i,j) = 0.
+    enddo
+350 continue
+  enddo
+end subroutine
+
+! No branching at all; detection must leave it unmarked.
+subroutine fully_structured(a, n)
+  real :: a(n)
+  ! CHECK:   <<DoConstruct>> -> 4
+  ! CHECK:   <<End DoConstruct>>
+  do i = 1, n
+    a(i) = 5.0
+  end do
+end subroutine
diff --git a/flang/test/Lower/trailing-cycle.f90 b/flang/test/Lower/trailing-cycle.f90
index 47f904cb46c99..4a69a3b4fc0ba 100644
--- a/flang/test/Lower/trailing-cycle.f90
+++ b/flang/test/Lower/trailing-cycle.f90
@@ -58,8 +58,10 @@ subroutine trailing_cycle(a, n)
     end do
   end do outer
 
-  ! A labeled CYCLE may be a branch target and is kept.
-  ! CHECK:   <<DoConstruct!>> -> 25
+  ! A labeled CYCLE may be a branch target and is kept. Its branches stay
+  ! inside the loop body, so the loop is category (c): structured control with
+  ! unstructured internals, marked '~' rather than '!'.
+  ! CHECK:   <<DoConstruct~>> -> 25
   ! CHECK:     18 NonLabelDoStmt -> 24: do i = 1, n
   ! CHECK:     <<IfConstruct>> -> 23
   ! CHECK:       19 ^IfStmt [negate] -> 23: if(a(i) > 0.0) goto 10
@@ -68,20 +70,21 @@ subroutine trailing_cycle(a, n)
   ! CHECK:     <<End IfConstruct>>
   ! CHECK:     23 CycleStmt! -> 24: 10 cycle
   ! CHECK:     24 ^EndDoStmt -> 18 <- 23: end do
-  ! CHECK:   <<End DoConstruct!>>
+  ! CHECK:   <<End DoConstruct~>>
   do i = 1, n
     if (a(i) > 0.0) goto 10
     a(i) = 4.0
 10  cycle
   end do
 
-  ! A CYCLE that is not last is a real branch and is kept.
-  ! CHECK:   <<DoConstruct!>> -> 29
+  ! A CYCLE that is not last is a real branch and is kept. It targets the
+  ! EndDoStmt, which is the loop-body boundary, so this is category (c) too.
+  ! CHECK:   <<DoConstruct~>> -> 29
   ! CHECK:     25 ^NonLabelDoStmt -> 28: do i = 1, n
   ! CHECK:     26 ^CycleStmt! -> 28: cycle
   ! CHECK:     27 ^AssignmentStmt: a(i) = 5.0
   ! CHECK:     28 ^EndDoStmt -> 25 <- 26: end do
-  ! CHECK:   <<End DoConstruct!>>
+  ! CHECK:   <<End DoConstruct~>>
   do i = 1, n
     cycle
     a(i) = 5.0

>From 75cf746e28673d7e3c026cd665422e0bc93204a7 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at gmail.com>
Date: Fri, 25 Sep 2026 00:59:43 -0700
Subject: [PATCH 2/5] [flang] Honor the execute-region wrap flag when detecting
 loop internals

A loop whose branching is confined to its body is lowered with that body
in an scf.execute_region, since the CFG needs more than the single block
fir.do_loop's region admits. With the wrap disabled there is nowhere to
put those blocks, so skip the reclassification and leave the loop
unstructured.
---
 flang/lib/Lower/PFTBuilder.cpp | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/flang/lib/Lower/PFTBuilder.cpp b/flang/lib/Lower/PFTBuilder.cpp
index bc66c80aa68fe..4d1b7588616d6 100644
--- a/flang/lib/Lower/PFTBuilder.cpp
+++ b/flang/lib/Lower/PFTBuilder.cpp
@@ -2901,6 +2901,13 @@ static bool isStructurableWithUnstructuredInternals(
 /// so leaving the parent Unstructured is conservative but correct.
 static void detectStructuredWithUnstructuredInternals(
     Fortran::lower::pft::FunctionLikeUnit &unit) {
+  // Such a loop is lowered with its body in an scf.execute_region: its
+  // branching needs more than the one block fir.do_loop's region admits. With
+  // the wrap disabled there is nowhere to put that CFG, so leave the loop
+  // Unstructured and lower its branches as they are.
+  if (!wrapUnstructuredConstructsInExecuteRegion)
+    return;
+
   std::function<void(Fortran::lower::pft::EvaluationList &)> visit =
       [&](Fortran::lower::pft::EvaluationList &list) {
         for (Fortran::lower::pft::Evaluation &e : list) {

>From 8f7b3eecd5da3219d4ed4429f0907c9b249d7cbb Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at gmail.com>
Date: Fri, 25 Sep 2026 01:04:03 -0700
Subject: [PATCH 3/5] [flang] Keep loops with a non-terminating body
 unstructured

A loop body that cannot run to completion must not be folded into an
scf.execute_region: the region carries no memory effects, so DCE deletes
it outright, dropping the non-termination and letting execution fall past
the loop. Branches survive that, being terminators.

An infinite DO was already rejected. Follow chains of unconditional GO TOs
as well and reject a body whose chain closes on itself, which is the same
bound the cf.br canonicalization applies to cyclic branches.
---
 flang/lib/Lower/PFTBuilder.cpp                | 31 +++++++++++-
 .../Lower/do-loop-infinite-body-cycle.f90     | 49 +++++++++++++++++++
 2 files changed, 79 insertions(+), 1 deletion(-)
 create mode 100644 flang/test/Lower/do-loop-infinite-body-cycle.f90

diff --git a/flang/lib/Lower/PFTBuilder.cpp b/flang/lib/Lower/PFTBuilder.cpp
index 4d1b7588616d6..7ab91d22799d1 100644
--- a/flang/lib/Lower/PFTBuilder.cpp
+++ b/flang/lib/Lower/PFTBuilder.cpp
@@ -2800,6 +2800,26 @@ static bool isInLoopBody(const Fortran::lower::pft::Evaluation *eval,
 /// A CYCLE is not an escape: its target is the EndDoStmt, which is where the
 /// wrap's yield sits, so it lands on the boundary. An EXIT targets the
 /// construct exit, beyond the loop entirely, and does escape.
+/// Follow the chain of unconditional GO TOs starting at \p start and return
+/// true if it closes on itself.
+///
+/// Such a cycle has no exit edge, which makes it a statically known infinite
+/// loop. Only unconditional transfers are followed, so the answer is a
+/// certainty rather than a guess -- the same bound the cf.br canonicalization
+/// applies when it declines to collapse cyclic branches.
+static bool
+startsExitFreeGotoCycle(const Fortran::lower::pft::Evaluation &start) {
+  auto gotoTarget = [](const Fortran::lower::pft::Evaluation &e) {
+    return e.getIf<parser::GotoStmt>() ? e.controlSuccessor : nullptr;
+  };
+
+  llvm::SmallPtrSet<const Fortran::lower::pft::Evaluation *, 4> visited;
+  for (const Fortran::lower::pft::Evaluation *e = &start; e; e = gotoTarget(*e))
+    if (!visited.insert(e).second)
+      return true;
+  return false;
+}
+
 static bool isStructurableWithUnstructuredInternals(
     const Fortran::lower::pft::Evaluation &loop,
     const Fortran::lower::pft::FunctionLikeUnit &unit) {
@@ -2850,8 +2870,17 @@ static bool isStructurableWithUnstructuredInternals(
 
   std::function<bool(const Fortran::lower::pft::Evaluation &)> check =
       [&](const Fortran::lower::pft::Evaluation &e) -> bool {
+    // A body that cannot run to completion must stay unstructured. Its
+    // structured form puts the body in an scf.execute_region carrying no
+    // memory effects, and DCE deletes such a region outright -- discarding the
+    // non-termination and letting execution fall past the loop. Branches
+    // survive that, being terminators, so leave the loop unstructured.
+    //
+    // An infinite DO says so in its own syntax; a GO TO cycle has to be
+    // followed to be recognized.
     if (e.isA<parser::ReturnStmt>() ||
-        isInfiniteDo(e.getIf<parser::DoConstruct>()))
+        isInfiniteDo(e.getIf<parser::DoConstruct>()) ||
+        startsExitFreeGotoCycle(e))
       return false;
 
     if (const auto *g = e.getIf<parser::AssignedGotoStmt>())
diff --git a/flang/test/Lower/do-loop-infinite-body-cycle.f90 b/flang/test/Lower/do-loop-infinite-body-cycle.f90
new file mode 100644
index 0000000000000..8914690084e34
--- /dev/null
+++ b/flang/test/Lower/do-loop-infinite-body-cycle.f90
@@ -0,0 +1,49 @@
+! Check that a loop whose body contains a statically known infinite loop is not
+! reclassified. Its structured form would place the body in an
+! scf.execute_region with no memory effects, which DCE deletes outright --
+! dropping the non-termination and letting execution continue past the loop.
+!
+! `!` marks a loop left unstructured, `~` one whose branching is confined to
+! its body.
+
+! RUN: %flang_fc1 -fdebug-dump-pft -o /dev/null %s 2>&1 | FileCheck %s
+
+! A GO TO branching to itself never leaves the body.
+subroutine self_cycle(n)
+  integer :: n, i
+  do i = 1, n
+10   goto 10
+  end do
+  call never_executed()
+end subroutine
+
+! CHECK: Subroutine self_cycle
+! CHECK: <<DoConstruct!>>
+
+! Two GO TOs branching to each other form the same exit-free cycle.
+subroutine mutual_cycle(n)
+  integer :: n, i
+  do i = 1, n
+20   goto 30
+30   goto 20
+  end do
+  call never_executed()
+end subroutine
+
+! CHECK: Subroutine mutual_cycle
+! CHECK: <<DoConstruct!>>
+
+! Control: nothing branches here at all. The ASSIGN alone makes label 41 a
+! branch target, which is what gives the body a block of its own, and with no
+! branch there is nothing to trap control. The loop still qualifies.
+subroutine label_target_in_body(a, b)
+  real :: a(10), b(10)
+  integer :: m
+  do 42 i = 1, 10
+    assign 41 to m
+41  a(i) = b(i)
+42 continue
+end subroutine
+
+! CHECK: Subroutine label_target_in_body
+! CHECK: <<DoConstruct~>>

>From 60aabe20b2659feb75217d5b47000c15a3579e10 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at gmail.com>
Date: Mon, 28 Sep 2026 12:26:04 -0700
Subject: [PATCH 4/5] [flang] Collect ASSIGNed labels before analyzing branches

An assigned GO TO reaches every label ASSIGNed to its variable, wherever the
ASSIGN sits. Branch analysis marked only the labels it had already walked
past, so an ASSIGN written after the GO TO left that target unrecorded. The
successors, and the incoming-branch map built from them, were incomplete for
every reader.

Collect the ASSIGNed labels of a unit before its branches are analyzed. The
recorded successors then name every target, so an assigned GO TO outside a
loop that can enter its body is seen as entering it, and a loop is no longer
reclassified as having self-contained branching when it has not.

With the successors complete, the classification needs no special case for
these statements: a listless GO TO is decided on its targets like any other
branch, rather than being turned away because a label list did not bound
them.
---
 flang/lib/Lower/PFTBuilder.cpp                | 38 ++++++++++++---
 .../test/Lower/pre-fir-tree-assigned-goto.f90 | 47 +++++++++++++++++++
 .../Lower/pre-fir-tree-branch-into-body.f90   | 43 +++++++++++++++++
 3 files changed, 121 insertions(+), 7 deletions(-)
 create mode 100644 flang/test/Lower/pre-fir-tree-branch-into-body.f90

diff --git a/flang/lib/Lower/PFTBuilder.cpp b/flang/lib/Lower/PFTBuilder.cpp
index 7ab91d22799d1..9173eed18b2bb 100644
--- a/flang/lib/Lower/PFTBuilder.cpp
+++ b/flang/lib/Lower/PFTBuilder.cpp
@@ -527,11 +527,39 @@ class PFTBuilder {
     return true;
   }
 
+  /// Record every label ASSIGNed in the unit, before branches are analyzed.
+  ///
+  /// An assigned GO TO reaches the labels ASSIGNed to its variable anywhere in
+  /// the unit, including in statements that follow it. Collecting them up front
+  /// lets analyzeBranches mark all of those targets rather than only the ones
+  /// the walk has already passed.
+  void collectAssignedLabels(lower::pft::EvaluationList &evaluationList) {
+    for (lower::pft::Evaluation &eval : evaluationList) {
+      if (const auto *s = eval.getIf<parser::AssignStmt>()) {
+        parser::Label label = std::get<parser::Label>(s->t);
+        if (const semantics::Symbol *sym =
+                std::get<parser::Name>(s->t).symbol) {
+          auto iter = assignSymbolLabelMap->find(*sym);
+          if (iter == assignSymbolLabelMap->end()) {
+            lower::pft::LabelSet labelSet{};
+            labelSet.insert(label);
+            assignSymbolLabelMap->try_emplace(*sym, labelSet);
+          } else {
+            iter->second.insert(label);
+          }
+        }
+      }
+      if (eval.evaluationList)
+        collectAssignedLabels(*eval.evaluationList);
+    }
+  }
+
   void exitFunction() {
     lower::pft::FunctionLikeUnit *exitingUnit = currentFunctionUnit;
     currentFunctionUnit = nullptr; // Clear when exiting function
     rewriteIfGotos();
     endFunctionBody();
+    collectAssignedLabels(*evaluationListStack.back());
     analyzeBranches(nullptr, *evaluationListStack.back()); // add branch links
 
     // Branch analysis is complete, so the incoming-branch map is too.
@@ -1139,9 +1167,9 @@ class PFTBuilder {
             };
             for (const auto &label : std::get<std::list<parser::Label>>(s.t))
               markIfBranchTarget(label);
-            // TODO: This may miss assignments that appear later in program
-            // order, but it matches the information available at this point in
-            // the walk.
+            // collectAssignedLabels filled this map before the walk started,
+            // so it names every label ASSIGNed to the variable, including by
+            // statements this GO TO has not reached yet.
             if (const auto *sym = std::get<parser::Name>(s.t).symbol) {
               auto iter = assignSymbolLabelMap->find(*sym);
               if (iter != assignSymbolLabelMap->end())
@@ -2883,10 +2911,6 @@ static bool isStructurableWithUnstructuredInternals(
         startsExitFreeGotoCycle(e))
       return false;
 
-    if (const auto *g = e.getIf<parser::AssignedGotoStmt>())
-      if (std::get<std::list<parser::Label>>(g->t).empty())
-        return false;
-
     // Condition 1: nothing leaves the body, CYCLE excepted.
     if (e.controlSuccessor && targetEscapes(e.controlSuccessor))
       return false;
diff --git a/flang/test/Lower/pre-fir-tree-assigned-goto.f90 b/flang/test/Lower/pre-fir-tree-assigned-goto.f90
index 757a838d592b8..cf0332aee2d8b 100644
--- a/flang/test/Lower/pre-fir-tree-assigned-goto.f90
+++ b/flang/test/Lower/pre-fir-tree-assigned-goto.f90
@@ -45,3 +45,50 @@ subroutine assigned_goto_repeated_label(j)
 10 print *, "ten"
 20 print *, "twenty"
 end subroutine
+
+! An assigned GO TO reaches every label ASSIGNed to its variable, wherever the
+! ASSIGN sits. The labels are collected before branches are analyzed, so one
+! written after the GO TO is recorded as a target like any other.
+
+! The GO TO carries no label list, so the ASSIGNs are all that name its
+! targets. Both labels ASSIGN'd to m lie in the loop body, so the branching is
+! self-contained and the loop keeps its structured form -- a listless GO TO is
+! decided on its targets like any other. The ASSIGN of label 10 follows the GO
+! TO, and label 10 is a recorded successor all the same: index 4 carries its
+! "<-" edge.
+! CHECK-LABEL: Subroutine targets_inside
+! CHECK: <<DoConstruct~>>
+! CHECK: [[GOTO:[0-9]+]] ^AssignedGotoStmt! -> [[L20:[0-9]+]], [[L10:[0-9]+]]: go to m
+! CHECK: [[L10]] ^AssignmentStmt <- [[GOTO]]: 10 a(i) = 1.0
+! CHECK: [[L20]] ^AssignmentStmt <- [[GOTO]]: 20 a(i) = a(i) + 1.0
+! CHECK: <<End DoConstruct~>>
+subroutine targets_inside(a, n)
+  real :: a(n)
+  integer :: m
+  assign 20 to m
+  do i = 1, n
+    go to m
+10  a(i) = 1.0
+20  a(i) = a(i) + 1.0
+    assign 10 to m
+  end do
+end subroutine
+
+! Also listless, and the only ASSIGN follows the GO TO. Label 30 lies outside
+! the loop, so a branch can leave the body and the loop stays unstructured --
+! which is visible only because that late ASSIGN is collected: with no target
+! recorded at all, nothing would appear to leave the body.
+! CHECK-LABEL: Subroutine target_outside
+! CHECK: <<DoConstruct!>>
+! CHECK: ^AssignedGotoStmt! -> {{[0-9]+}}: go to m
+! CHECK: <<End DoConstruct!>>
+subroutine target_outside(a, n)
+  real :: a(n)
+  integer :: m
+  do i = 1, n
+    go to m
+10  a(i) = 1.0
+    assign 30 to m
+  end do
+30 continue
+end subroutine
diff --git a/flang/test/Lower/pre-fir-tree-branch-into-body.f90 b/flang/test/Lower/pre-fir-tree-branch-into-body.f90
new file mode 100644
index 0000000000000..15ddb63d3384c
--- /dev/null
+++ b/flang/test/Lower/pre-fir-tree-branch-into-body.f90
@@ -0,0 +1,43 @@
+! A loop qualifies only when its body branching is self-contained. An assigned
+! GO TO reaches every label ASSIGNed to its variable, so one outside a loop can
+! enter its body -- including through an ASSIGN written after the GO TO itself.
+!
+! `!` marks a loop left unstructured, `~` one whose branching is confined to
+! its body.
+
+! RUN: %flang_fc1 -fdebug-dump-pft -o /dev/null %s 2>&1 | FileCheck %s
+
+! The ASSIGN that puts a body label into the variable comes after the assigned
+! GO TO. Collecting the ASSIGNed labels before branches are analyzed records
+! label 20 as a target, so the branch into the body is seen.
+subroutine assigned_goto_reenters(a, n)
+  real :: a(n)
+  integer :: m, i, n
+  assign 10 to m
+5 go to m
+10 continue
+  do i = 1, n
+    a(i) = 1.0
+    assign 20 to m
+20  a(i) = a(i) + 1.0
+  end do
+  goto 5
+end subroutine
+
+! CHECK: Subroutine assigned_goto_reenters
+! CHECK: ^AssignedGotoStmt!
+! CHECK: <<DoConstruct!>>
+
+! Control: an ASSIGN naming a body label is not itself a branch. With no
+! assigned GO TO to use it, nothing can enter the body and the loop qualifies.
+subroutine assign_without_goto(a, b)
+  real :: a(10), b(10)
+  integer :: m
+  do 42 i = 1, 10
+    assign 41 to m
+41  a(i) = b(i)
+42 continue
+end subroutine
+
+! CHECK: Subroutine assign_without_goto
+! CHECK: <<DoConstruct~>>

>From d49835623395039fcec15aea6ea45c48b2776fb4 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at gmail.com>
Date: Tue, 29 Sep 2026 02:58:47 -0700
Subject: [PATCH 5/5] [flang] Search every way back to a GO TO when looking for
 a cycle

A GO TO reaching a label above it closes a cycle, but the way back need not be
a branch: the statement it lands on simply runs on, through the constructs it
meets, until control reaches the GO TO again. Chaining from one GO TO to the
next stopped at the first statement of another kind and missed the cycle,
leaving the body free to run forever inside a region DCE then deleted.

Follow every successor instead, from the GO TO's targets until control returns
to it. An assigned GO TO closes a cycle the same way, and its targets are
already named by its successors.

Two edges are left out of the search. It stays inside the loop body, since
beyond it lies the loop's own iteration edge, which would carry the search back
in and make every branch look cyclic. For the same reason it skips the
iteration edge of any loop nested in that body: a loop reaching its own DO
statement ends on its own control.

The answer is an over-approximation. Some successor leads back, but control
need not take it -- a conditional branch out of the cycle makes the loop finish
after all. Such a loop is turned away too, costing it its structured form and
nothing else.
---
 flang/lib/Lower/PFTBuilder.cpp                |  87 +++++++++----
 .../Lower/do-loop-infinite-body-cycle.f90     | 121 ++++++++++++++++++
 2 files changed, 186 insertions(+), 22 deletions(-)

diff --git a/flang/lib/Lower/PFTBuilder.cpp b/flang/lib/Lower/PFTBuilder.cpp
index 9173eed18b2bb..9e0181396b52f 100644
--- a/flang/lib/Lower/PFTBuilder.cpp
+++ b/flang/lib/Lower/PFTBuilder.cpp
@@ -1138,14 +1138,6 @@ class PFTBuilder {
                   parent->constructExit->isNewBlock = true;
               }
             }
-            auto iter = assignSymbolLabelMap->find(*sym);
-            if (iter == assignSymbolLabelMap->end()) {
-              lower::pft::LabelSet labelSet{};
-              labelSet.insert(label);
-              assignSymbolLabelMap->try_emplace(*sym, labelSet);
-            } else {
-              iter->second.insert(label);
-            }
           },
           [&](const parser::AssignedGotoStmt &s) {
             // See Fortran 90 Clause 8.2.4.
@@ -2828,23 +2820,74 @@ static bool isInLoopBody(const Fortran::lower::pft::Evaluation *eval,
 /// A CYCLE is not an escape: its target is the EndDoStmt, which is where the
 /// wrap's yield sits, so it lands on the boundary. An EXIT targets the
 /// construct exit, beyond the loop entirely, and does escape.
-/// Follow the chain of unconditional GO TOs starting at \p start and return
-/// true if it closes on itself.
+/// Return true if control can get from where \p start branches back to \p
+/// start itself, without leaving \p loop's body.
 ///
-/// Such a cycle has no exit edge, which makes it a statically known infinite
-/// loop. Only unconditional transfers are followed, so the answer is a
-/// certainty rather than a guess -- the same bound the cf.br canonicalization
-/// applies when it declines to collapse cyclic branches.
-static bool
-startsExitFreeGotoCycle(const Fortran::lower::pft::Evaluation &start) {
-  auto gotoTarget = [](const Fortran::lower::pft::Evaluation &e) {
-    return e.getIf<parser::GotoStmt>() ? e.controlSuccessor : nullptr;
+/// A branch closes a cycle with whatever carries control back to it, and that
+/// return path is made of ordinary statements: the one branched to simply runs
+/// on, through the constructs it meets, until it reaches the branch again.
+/// Following a single path would lose the way back wherever control could go
+/// more than one way, so every successor is followed.
+///
+/// A cycle may run forever, which disqualifies the loop holding it: its
+/// structured form puts the body in an scf.execute_region carrying no memory
+/// effects, and DCE deletes such a region outright. Whether the cycle can be
+/// left is not checked, so a loop that does terminate is rejected as well.
+///
+/// The search stays inside the body. Beyond it lies the loop's own iteration
+/// edge, from the EndDoStmt back to the DO statement, which would carry the
+/// search back into the body and make every branch look like a cycle.
+static bool startsBranchCycle(const Fortran::lower::pft::Evaluation &start,
+                              const Fortran::lower::pft::Evaluation &loop) {
+  // Only a GO TO starts the search. A loop reaches its own DO statement from
+  // its EndDoStmt on every iteration, and asking of either would find that
+  // edge and call the loop holding it non-terminating.
+  if (!start.getIf<parser::GotoStmt>() &&
+      !start.getIf<parser::AssignedGotoStmt>())
+    return false;
+
+  // Every way control may leave \p e: where it branches, and the statement
+  // after it.
+  // A loop reaches its own DO statement from its EndDoStmt on every iteration.
+  // That edge would lead the search back through the loop's body and make any
+  // branch inside it look like a cycle, so it is skipped: the loop terminates
+  // through its own control.
+  auto isOwnIterationEdge = [](const Fortran::lower::pft::Evaluation &e,
+                               const Fortran::lower::pft::Evaluation *target) {
+    const Fortran::lower::pft::Evaluation *parent = e.parentConstruct;
+    return parent && parent->getIf<parser::DoConstruct>() &&
+           parent->evaluationList && &parent->evaluationList->back() == &e &&
+           &parent->evaluationList->front() == target;
   };
 
-  llvm::SmallPtrSet<const Fortran::lower::pft::Evaluation *, 4> visited;
-  for (const Fortran::lower::pft::Evaluation *e = &start; e; e = gotoTarget(*e))
-    if (!visited.insert(e).second)
+  auto successors =
+      [&](const Fortran::lower::pft::Evaluation &e,
+          llvm::SmallVectorImpl<const Fortran::lower::pft::Evaluation *> &out) {
+        if (e.controlSuccessor && !isOwnIterationEdge(e, e.controlSuccessor))
+          out.push_back(e.controlSuccessor);
+        for (const Fortran::lower::pft::Evaluation *extra :
+             e.extraControlSuccessors)
+          out.push_back(extra);
+        // A GO TO reaches only its target; the statement it precedes is not a
+        // successor of it. An assigned GO TO reaches every label ASSIGNed to
+        // its variable, which the successors already name.
+        if (!e.getIf<parser::GotoStmt>() &&
+            !e.getIf<parser::AssignedGotoStmt>() && e.lexicalSuccessor)
+          out.push_back(e.lexicalSuccessor);
+      };
+
+  llvm::SmallVector<const Fortran::lower::pft::Evaluation *> worklist;
+  successors(start, worklist);
+
+  llvm::SmallPtrSet<const Fortran::lower::pft::Evaluation *, 16> seen;
+  while (!worklist.empty()) {
+    const Fortran::lower::pft::Evaluation *e = worklist.pop_back_val();
+    if (e == &start)
       return true;
+    if (!isInLoopBody(e, loop) || !seen.insert(e).second)
+      continue;
+    successors(*e, worklist);
+  }
   return false;
 }
 
@@ -2908,7 +2951,7 @@ static bool isStructurableWithUnstructuredInternals(
     // followed to be recognized.
     if (e.isA<parser::ReturnStmt>() ||
         isInfiniteDo(e.getIf<parser::DoConstruct>()) ||
-        startsExitFreeGotoCycle(e))
+        startsBranchCycle(e, loop))
       return false;
 
     // Condition 1: nothing leaves the body, CYCLE excepted.
diff --git a/flang/test/Lower/do-loop-infinite-body-cycle.f90 b/flang/test/Lower/do-loop-infinite-body-cycle.f90
index 8914690084e34..6ee7334e2f507 100644
--- a/flang/test/Lower/do-loop-infinite-body-cycle.f90
+++ b/flang/test/Lower/do-loop-infinite-body-cycle.f90
@@ -20,6 +20,64 @@ subroutine self_cycle(n)
 ! CHECK: Subroutine self_cycle
 ! CHECK: <<DoConstruct!>>
 
+! The way back to the GO TO is not a branch: control reaches the CONTINUE and
+! then continues to the GO TO again. The cycle is found by following control
+! from the target, not by looking for another GO TO.
+subroutine cycle_through_continue(n)
+  integer :: n, i
+  do i = 1, n
+10  continue
+    goto 10
+  end do
+  call never_executed()
+end subroutine
+
+! CHECK: Subroutine cycle_through_continue
+! CHECK: <<DoConstruct!>>
+
+! Control: the same GO TO pointing forwards. Control leaves the body through
+! the EndDoStmt without passing back through it, so the loop qualifies.
+subroutine forward_goto(a, n)
+  real :: a(n)
+  integer :: n, i
+  do i = 1, n
+    if (a(i) > 0.0) then
+      a(i) = 1.0
+      goto 90
+    end if
+    a(i) = 2.0
+90  continue
+  end do
+end subroutine
+
+! CHECK: Subroutine forward_goto
+! CHECK: <<DoConstruct~>>
+
+! Control: a nested loop returns to its own DO statement on every iteration.
+! That is the loop's own control rather than a way back to the GO TO, so it
+! does not disqualify the loop holding it.
+subroutine nested_loop_iterates(a, n)
+  real :: a(n,n)
+  integer :: n, i, j
+  do i = 1, n
+    do j = 1, n
+      a(i,j) = 0.0
+    end do
+    if (a(i,1) > 0.0) then
+      a(i,1) = 2.0
+      goto 90
+    end if
+    a(i,1) = 1.0
+90  continue
+  end do
+end subroutine
+
+! CHECK: Subroutine nested_loop_iterates
+! CHECK: <<DoConstruct~>>
+! CHECK: <<DoConstruct>>
+! CHECK: <<End DoConstruct>>
+! CHECK: <<End DoConstruct~>>
+
 ! Two GO TOs branching to each other form the same exit-free cycle.
 subroutine mutual_cycle(n)
   integer :: n, i
@@ -47,3 +105,66 @@ subroutine label_target_in_body(a, b)
 
 ! CHECK: Subroutine label_target_in_body
 ! CHECK: <<DoConstruct~>>
+
+! The way back runs through a construct. Control leaves the CONTINUE, passes
+! through the IF, and reaches the GO TO again, so following every successor
+! rather than a single path is what finds it. The body holds nothing else, so
+! its region would carry no memory effects and DCE would delete it outright.
+subroutine cycle_through_construct(n)
+  integer :: n, i
+  do i = 1, n
+10  continue
+    if (n > 0) then
+    end if
+    goto 10
+  end do
+  call never_executed()
+end subroutine
+
+! CHECK: Subroutine cycle_through_construct
+! CHECK: <<DoConstruct!>>
+
+! An assigned GO TO closes a cycle like any other branch: its targets are the
+! labels ASSIGNed to its variable, which its successors already name.
+subroutine assigned_goto_cycle(n)
+  integer :: n, i, m
+  do i = 1, n
+10  continue
+    assign 10 to m
+    go to m
+  end do
+  call never_executed()
+end subroutine
+
+! CHECK: Subroutine assigned_goto_cycle
+! CHECK: <<DoConstruct!>>
+
+! Control: the inner loop's iteration edge is on the path back to the GO TO.
+! Following it would report a cycle, although both loops terminate.
+!
+! The ASSIGN is needed. It keeps the outer loop unstructured, so the outer loop
+! is the one analysed. Without it, only the inner loop is analysed, and its
+! EndDoStmt is outside its own body, so the search stops before it reaches an
+! iteration edge.
+subroutine goto_inside_nested_loop(a, n)
+  real :: a(n,n)
+  integer :: n, i, j, m
+  do i = 1, n
+    assign 80 to m
+    do j = 1, n
+      if (a(i,j) > 0.0) then
+        a(i,j) = 1.0
+        goto 70
+      end if
+      a(i,j) = 2.0
+70    continue
+    end do
+80  continue
+  end do
+end subroutine
+
+! CHECK: Subroutine goto_inside_nested_loop
+! CHECK: <<DoConstruct~>>
+! CHECK: <<DoConstruct~>>
+! CHECK: <<End DoConstruct~>>
+! CHECK: <<End DoConstruct~>>



More information about the flang-commits mailing list