[flang-commits] [flang] [flang] Rewrite boxproc ops inside module-level regions with -fsafe-trampoline (PR #232167)

[Eugene Epshteyn via flang-commits]

https://github.com/eugeneepshteyn created https://github.com/llvm/llvm-project/pull/232167

With `-fsafe-trampoline`, `BoxedProcedurePass` processes functions one by one (so that `TrampolineFree` can be inserted before each return) and then visits the remaining module-level operations — but only those top-level operations themselves, never the operations nested in their regions. A module or `SAVE` procedure pointer with an initializer is a `fir.global` whose initializer region contains a `fir.emboxproc`; that op survived the pass and codegen aborted with `not yet implemented: fir.emboxproc codegen` for any such procedure pointer, whether or not an internal procedure or a trampoline is involved. The default path's recursive module walk does not have this gap.

Walk the regions of the non-function module-level operations the way the default path does, skipping functions and their contents since they have already been processed (including functions nested in a `gpu.module`).

Tests: a `fir-opt` regression over `fir.global` initializers with null and non-null procedure targets, checked in both modes, and an integration test compiling module procedure pointers with `-fsafe-trampoline` to LLVM IR.

Fixes #223705.


>From 2d4123bcd9db132cad77fbe7ac5c062667295751 Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Wed, 14 Oct 2026 19:45:41 -0700
Subject: [PATCH] [flang] Rewrite boxproc ops inside module-level regions with
 -fsafe-trampoline

With -fsafe-trampoline, BoxedProcedurePass processes functions one by one
(so that TrampolineFree can be inserted before each return) and then
visits the remaining module-level operations. It visited only those
top-level operations themselves, never the operations nested in their
regions. A module or SAVE procedure pointer with an initializer is a
fir.global whose initializer region contains a fir.emboxproc, so that op
survived the pass and codegen aborted with "not yet implemented:
fir.emboxproc codegen" for any such procedure pointer, whether or not an
internal procedure or a trampoline is involved. The default path's
recursive module walk does not have this gap.

Walk the regions of the non-function module-level operations the way the
default path does, skipping functions and their contents since they have
already been processed (including functions nested in a gpu.module).

This fixes issue 223705.
---
 .../lib/Optimizer/CodeGen/BoxedProcedure.cpp  | 14 +++++-
 .../Fir/boxproc-safe-trampoline-global.fir    | 48 +++++++++++++++++++
 .../safe-trampoline-procptr-global.f90        | 32 +++++++++++++
 3 files changed, 92 insertions(+), 2 deletions(-)
 create mode 100644 flang/test/Fir/boxproc-safe-trampoline-global.fir
 create mode 100644 flang/test/Integration/safe-trampoline-procptr-global.f90

diff --git a/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp b/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp
index 1ec365664b2958..535a1f629d62de 100644
--- a/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp
+++ b/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp
@@ -258,12 +258,22 @@ class BoxedProcedurePass
         [&](mlir::Operation *op) { processOp(op, rewriter, typeConverter); });
   }
 
-  /// Process non-function ops at module level (globals, etc.)
+  /// Process non-function ops at module level (globals, etc.), including the
+  /// ops nested in their regions: a procedure pointer with an initializer is a
+  /// fir.global whose initializer region contains a fir.emboxproc, and that
+  /// op has no codegen, so it must be rewritten here like the default path's
+  /// recursive walk does. Functions were already processed one by one above
+  /// (also those nested in a gpu.module), so their contents are skipped.
   void processModuleLevelOps(mlir::IRRewriter &rewriter,
                              BoxprocTypeRewriter &typeConverter) {
     for (auto &op : getModule().getBody()->getOperations())
       if (!mlir::isa<mlir::func::FuncOp>(op))
-        processOp(&op, rewriter, typeConverter);
+        op.walk([&](mlir::Operation *nested) {
+          if (mlir::isa<mlir::func::FuncOp>(nested) ||
+              nested->getParentOfType<mlir::func::FuncOp>())
+            return;
+          processOp(nested, rewriter, typeConverter);
+        });
   }
 
   /// Insert _FortranATrampolineFree calls before every return in the function.
diff --git a/flang/test/Fir/boxproc-safe-trampoline-global.fir b/flang/test/Fir/boxproc-safe-trampoline-global.fir
new file mode 100644
index 00000000000000..ba0161c4ed1e14
--- /dev/null
+++ b/flang/test/Fir/boxproc-safe-trampoline-global.fir
@@ -0,0 +1,48 @@
+// RUN: fir-opt --boxed-procedure="use-safe-trampoline=true" %s | FileCheck %s
+// RUN: fir-opt --boxed-procedure %s | FileCheck %s
+
+// A module or SAVE procedure pointer with an initializer is lowered to a
+// fir.global whose initializer region emboxes a procedure. The boxed-procedure
+// pass must rewrite the ops inside that region in both modes: fir.emboxproc has
+// no codegen, and the safe-trampoline path used to visit only the fir.global
+// op itself, leaving the initializer untouched and the global's type
+// inconsistent with its fir.has_value.
+
+// CHECK-LABEL: fir.global @_QMmEnull_ptr : () -> () {
+// CHECK:         %[[ZERO:.*]] = fir.zero_bits () -> ()
+// CHECK:         %[[NULL:.*]] = fir.convert %[[ZERO]] : (() -> ()) -> (() -> ())
+// CHECK:         fir.has_value %[[NULL]] : () -> ()
+// CHECK-NOT:     fir.emboxproc
+fir.global @_QMmEnull_ptr : !fir.boxproc<() -> ()> {
+  %0 = fir.zero_bits () -> ()
+  %1 = fir.emboxproc %0 : (() -> ()) -> !fir.boxproc<() -> ()>
+  fir.has_value %1 : !fir.boxproc<() -> ()>
+}
+
+// CHECK-LABEL: fir.global @_QMmEtarget_ptr : () -> () {
+// CHECK:         %[[ADDR:.*]] = fir.address_of(@_QMmPcallee) : () -> ()
+// CHECK:         %[[FUNC:.*]] = fir.convert %[[ADDR]] : (() -> ()) -> (() -> ())
+// CHECK:         fir.has_value %[[FUNC]] : () -> ()
+// CHECK-NOT:     fir.emboxproc
+fir.global @_QMmEtarget_ptr : !fir.boxproc<() -> ()> {
+  %0 = fir.address_of(@_QMmPcallee) : () -> ()
+  %1 = fir.emboxproc %0 : (() -> ()) -> !fir.boxproc<() -> ()>
+  fir.has_value %1 : !fir.boxproc<() -> ()>
+}
+
+// CHECK-LABEL: func.func @_QMmPcallee()
+func.func @_QMmPcallee() {
+  return
+}
+
+// CHECK-LABEL: func.func @_QMmPcall_through_ptr()
+// CHECK:         %[[REF:.*]] = fir.address_of(@_QMmEtarget_ptr) : !fir.ref<() -> ()>
+// CHECK:         %[[LOAD:.*]] = fir.load %[[REF]] : !fir.ref<() -> ()>
+// CHECK:         fir.call %[[LOAD]]() : () -> ()
+func.func @_QMmPcall_through_ptr() {
+  %0 = fir.address_of(@_QMmEtarget_ptr) : !fir.ref<!fir.boxproc<() -> ()>>
+  %1 = fir.load %0 : !fir.ref<!fir.boxproc<() -> ()>>
+  %2 = fir.box_addr %1 : (!fir.boxproc<() -> ()>) -> (() -> ())
+  fir.call %2() : () -> ()
+  return
+}
diff --git a/flang/test/Integration/safe-trampoline-procptr-global.f90 b/flang/test/Integration/safe-trampoline-procptr-global.f90
new file mode 100644
index 00000000000000..9afff1ba7483fb
--- /dev/null
+++ b/flang/test/Integration/safe-trampoline-procptr-global.f90
@@ -0,0 +1,32 @@
+!===----------------------------------------------------------------------===!
+! This directory can be used to add Integration tests involving multiple
+! stages of the compiler (for eg. from Fortran to LLVM IR). It should not
+! contain executable tests. We should only add tests here sparingly and only
+! if there is no other way to test. Repeat this message in each test that is
+! added to this directory and sub-directories.
+!===----------------------------------------------------------------------===!
+
+! RUN: %flang_fc1 -fsafe-trampoline -emit-llvm -o - %s | FileCheck %s
+!
+! A module procedure pointer with an initializer is a global whose initializer
+! emboxes a procedure. With -fsafe-trampoline the boxed-procedure pass used to
+! leave that fir.emboxproc in place, and codegen aborted with
+! "not yet implemented: fir.emboxproc codegen". No internal procedure or
+! trampoline is involved: the globals must simply lower to plain pointers.
+
+! CHECK: @_QMmEnull_ptr = {{.*}}global ptr null
+! CHECK: @_QMmEtarget_ptr = {{.*}}global ptr @_QMmPcallee
+! CHECK-NOT: TrampolineInit
+
+module m
+  implicit none
+  abstract interface
+    subroutine callback()
+    end subroutine
+  end interface
+  procedure(callback), pointer :: null_ptr => null()
+  procedure(callback), pointer :: target_ptr => callee
+contains
+  subroutine callee()
+  end subroutine
+end module