[flang-commits] [flang] [flang][PFT-to-MLIR] reset Evaluation blocks between entry-point passes (PR #210681)

[Kareem Ergawy via flang-commits]

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

>From e5e20031fbe83a53d86229b7fff97c24fcb6e5c0 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at gmail.com>
Date: Mon, 20 Jul 2026 03:19:16 -0700
Subject: [PATCH] [flang][PFT-to-MLIR] reset Evaluation blocks between
 entry-point passes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A subprogram with an alternate ENTRY is lowered by walking the shared
PFT once per entry.  Evaluation::block is populated during each walk
(top-level createEmptyBlocks and inside wrapUnstructuredConstruct's own
createEmptyBlocks), but the second pass previously inherited stale
pointers into the first entry's function/wrap regions.  For a wrappable
IfConstruct nested inside a structured container, the entry block that
genFIR(IfConstruct) starts before creating its scf.execute_region then
sent the builder into the previous entry's region, and the wrap plus
its inner load ended up in the wrong func:

    subroutine foo(a)
      integer a
    entry bar(a)
      if (a .eq. 1) then
        if (a .ne. 3) stop
      end if
    end subroutine

produced both scf.execute_region ops in _QPfoo, with an inner
fir.load referencing bar's %arg0 — 'fir.load' op using value defined
outside the region.

Fix: null every Evaluation::block in the shared PFT at the start of each
entry-point pass, so createEmptyBlocks fills a clean tree and later
reads (in particular the landing-pad reposition in genFIR(IfConstruct))
never see a pointer from a prior pass.
---
 flang/lib/Lower/Bridge.cpp                    | 22 ++++++++++-
 .../if_construct_execute_region_wrap.f90      | 37 +++++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 3679eea9a0955..cc39cfffb5afc 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -3288,7 +3288,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
 
     // Start a new block before generating the scf.execute_region op if needed.
     // This avoids generating the region directly after a terminator (e.g. a
-    // conditional branch from a parent construct).
+    // conditional branch from a parent construct).  A non-null firstStmt.block
+    // here is a landing pad the enclosing wrap's createEmptyBlocks pre-
+    // allocated for us — startNewFunction runs resetEvaluationBlocks before
+    // each entry-point pass, so we can trust the pointer isn't stale from a
+    // previous pass.
     if (Fortran::lower::pft::isWrappableConstruct(eval) &&
         eval.hasNestedEvaluations()) {
       Fortran::lower::pft::Evaluation &firstStmt =
@@ -6486,6 +6490,10 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     // with dummy_scope operands.
     resetRegisteredDummySymbols();
 
+    // Clear any Evaluation::block pointers left over from a previous
+    // entry-point pass over this shared PFT.
+    resetEvaluationBlocks(funit.evaluationList);
+
     // Create most function blocks in advance.
     createEmptyBlocks(funit.evaluationList);
 
@@ -6512,6 +6520,18 @@ class FirConverter : public Fortran::lower::AbstractConverter {
       genBranch(alternateEntryEval->lexicalSuccessor->block);
   }
 
+  /// Recursively null every Evaluation::block in \p evaluationList.  Called
+  /// once per entry-point pass over a shared PFT so a later pass never sees
+  /// block pointers that belong to a prior pass's function or wrap region.
+  void resetEvaluationBlocks(
+      std::list<Fortran::lower::pft::Evaluation> &evaluationList) {
+    for (Fortran::lower::pft::Evaluation &eval : evaluationList) {
+      eval.block = nullptr;
+      if (eval.evaluationList)
+        resetEvaluationBlocks(*eval.evaluationList);
+    }
+  }
+
   /// Create global blocks for the current function. This eliminates the
   /// distinction between forward and backward targets when generating
   /// branches. A block is "global" if it can be the target of a GOTO or
diff --git a/flang/test/Lower/if_construct_execute_region_wrap.f90 b/flang/test/Lower/if_construct_execute_region_wrap.f90
index d1474a3aef70e..6817966cb6c2d 100644
--- a/flang/test/Lower/if_construct_execute_region_wrap.f90
+++ b/flang/test/Lower/if_construct_execute_region_wrap.f90
@@ -31,3 +31,40 @@ subroutine wrapped_if_nested_with_stop(a, b)
 ! CHECK:           fir.unreachable
 ! CHECK:           scf.yield
 ! CHECK:         }
+
+! Subroutine with an alternate ENTRY point followed by a structured outer
+! IF that nests a wrappable inner IF.  The shared PFT is lowered once per
+! entry, and the inner IfConstruct's firstStmt.block field still points
+! at the previous entry's wrap-region block on the second pass.  Reading
+! that stale pointer used to move the builder into the previous entry's
+! function, so both wraps ended up in _QPfoo and _QPbar was left empty
+! with a load referencing an SSA value from the other function.  Fixed by
+! only starting firstStmt.block when it belongs to the current builder's
+! region.
+subroutine foo_with_entry(a)
+  integer a
+entry bar_with_entry(a)
+  if (a .eq. 1) then
+    if (a .ne. 3) stop
+  end if
+end subroutine
+
+! CHECK-LABEL: func.func @_QPfoo_with_entry
+! CHECK:         %[[FOO_A:.*]]:2 = hlfir.declare {{.*}}uniq_name = "_QFfoo_with_entryEa"
+! CHECK:         fir.if
+! CHECK:           scf.execute_region
+! CHECK:             fir.load %[[FOO_A]]#0
+! CHECK:             cf.cond_br
+! CHECK:             fir.call @_FortranAStopStatement
+! CHECK:             scf.yield
+! CHECK:           }
+
+! CHECK-LABEL: func.func @_QPbar_with_entry
+! CHECK:         %[[BAR_A:.*]]:2 = hlfir.declare {{.*}}uniq_name = "_QFfoo_with_entryEa"
+! CHECK:         fir.if
+! CHECK:           scf.execute_region
+! CHECK:             fir.load %[[BAR_A]]#0
+! CHECK:             cf.cond_br
+! CHECK:             fir.call @_FortranAStopStatement
+! CHECK:             scf.yield
+! CHECK:           }