[flang-commits] [flang] [flang] only instantiate required symbols from parent modules (PR #193689)

via flang-commits flang-commits at lists.llvm.org
Thu Apr 23 01:34:30 PDT 2026


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-semantics

Author: jeanPerier

<details>
<summary>Changes</summary>

Currently lowering is instantiating (creating fir.address_of/hlfir.declare) for all module variables from host module and submodules (for instance, in the new host_module_variable_instantiation.f90 test, a fir.address_of was generated the unused var2 inside the procedure foo).

This created a lot of noise (and in the worst cases, compile time performance issues), and also some extra complexity at least for OpenACC where the IR acc routine ended up referencing globals that are no actually needed, creating the need to copy them on the GPU or to have custom logic to ignore the globals.

This patch addresses this by doing a visit of the parse tree to detect the required symbols and only instantiate those.

---
Full diff: https://github.com/llvm/llvm-project/pull/193689.diff


9 Files Affected:

- (modified) flang/include/flang/Lower/PFTBuilder.h (+7) 
- (modified) flang/include/flang/Semantics/tools.h (+3) 
- (modified) flang/lib/Lower/Bridge.cpp (+15-12) 
- (modified) flang/lib/Lower/PFTBuilder.cpp (+69) 
- (modified) flang/lib/Semantics/tools.cpp (+1-1) 
- (modified) flang/test/Lower/OpenMP/target-map-complex.f90 (+1-1) 
- (modified) flang/test/Lower/c-interoperability.f90 (-2) 
- (added) flang/test/Lower/host_module_variable_instantiation.f90 (+19) 
- (added) flang/test/Lower/proc_pointer_hidden_by_generic.f90 (+33) 


``````````diff
diff --git a/flang/include/flang/Lower/PFTBuilder.h b/flang/include/flang/Lower/PFTBuilder.h
index 55a755acaaeb7..8da101a11e47e 100644
--- a/flang/include/flang/Lower/PFTBuilder.h
+++ b/flang/include/flang/Lower/PFTBuilder.h
@@ -608,6 +608,13 @@ VariableList getScopeVariableList(const Fortran::semantics::Scope &scope);
 /// depends on. \p symbol itself will be the last variable in the list.
 VariableList getDependentVariableList(const Fortran::semantics::Symbol &);
 
+struct FunctionLikeUnit;
+/// Create an ordered list of equivalence sets and variables from host
+/// [sub]module scopes of \p funit that are referenced in \p funit. This is
+/// used by lowering of module procedures (and their internal subprograms) to
+/// only instantiate referenced host module variables rather than all of them.
+VariableList getHostModuleVariableList(const FunctionLikeUnit &funit);
+
 void dump(VariableList &, std::string s = {}); // `s` is an optional dump label
 
 /// Function-like units may contain evaluations (executable statements),
diff --git a/flang/include/flang/Semantics/tools.h b/flang/include/flang/Semantics/tools.h
index 9f77d0ec5da2e..53248a2b358d6 100644
--- a/flang/include/flang/Semantics/tools.h
+++ b/flang/include/flang/Semantics/tools.h
@@ -127,6 +127,9 @@ bool IsSeparateModuleProcedureInterface(const Symbol *);
 bool HasAlternateReturns(const Symbol &);
 bool IsAutomaticallyDestroyed(const Symbol &);
 
+// Follow association until the first symbol without HostAssocDetails.
+const Symbol &FollowHostAssoc(const Symbol &);
+
 // Return an ultimate component of type that matches predicate, or nullptr.
 const Symbol *FindUltimateComponent(const DerivedTypeSpec &type,
     const std::function<bool(const Symbol &)> &predicate);
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 655edc8ffa7b3..4594a2f73426e 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -6592,18 +6592,21 @@ class FirConverter : public Fortran::lower::AbstractConverter {
 
     Fortran::lower::AggregateStoreMap storeMap;
 
-    // Map all containing submodule and module equivalences and variables, in
-    // case they are referenced. It might be better to limit this to variables
-    // that are actually referenced, although that is more complicated when
-    // there are equivalenced variables.
-    auto &scopeVariableListMap =
-        Fortran::lower::pft::getScopeVariableListMap(funit);
-    for (auto *scp = &scope.parent(); !scp->IsGlobal(); scp = &scp->parent())
-      if (scp->kind() == Fortran::semantics::Scope::Kind::Module)
-        for (const auto &var : Fortran::lower::pft::getScopeVariableList(
-                 *scp, scopeVariableListMap))
-          if (!var.isRuntimeTypeInfoData())
-            instantiateVar(var, storeMap);
+    // Map containing [sub]module equivalences and variables that are
+    // referenced in this function-like unit. Only referenced host module
+    // variables are instantiated to avoid generating IR for unused module
+    // variables. Equivalenced variables are handled via the aggregate store
+    // analysis performed on the host [sub]module scopes.
+    const bool hasHostModuleScope = [&]() {
+      for (auto *scp = &scope.parent(); !scp->IsGlobal(); scp = &scp->parent())
+        if (scp->kind() == Fortran::semantics::Scope::Kind::Module)
+          return true;
+      return false;
+    }();
+    if (hasHostModuleScope)
+      for (const auto &var :
+           Fortran::lower::pft::getHostModuleVariableList(funit))
+        instantiateVar(var, storeMap);
 
     // Map function equivalences and variables.
     mlir::Value primaryFuncResultStorage;
diff --git a/flang/lib/Lower/PFTBuilder.cpp b/flang/lib/Lower/PFTBuilder.cpp
index 437e70265de94..d1b689edbb6a5 100644
--- a/flang/lib/Lower/PFTBuilder.cpp
+++ b/flang/lib/Lower/PFTBuilder.cpp
@@ -1622,6 +1622,20 @@ struct SymbolDependenceAnalysis {
     analyze(symbol);
     finalize();
   }
+  /// Analyze the dependencies of a set of module variables that are host
+  /// associated (or use associated in host module scopes).
+  SymbolDependenceAnalysis(
+      const llvm::SetVector<const semantics::Symbol *> &moduleVariables) {
+    for (const semantics::Symbol *sym : moduleVariables)
+      analyzeLocalEquivalenceSets(sym->owner());
+    // Add all aggregate stores to the front of the variable list.
+    adjustSize(1);
+    for (auto st : stores)
+      layeredVarList[0].emplace_back(std::move(st));
+    for (const semantics::Symbol *sym : moduleVariables)
+      analyze(*sym);
+    finalize();
+  }
   Fortran::lower::pft::VariableList getVariableList() {
     return std::move(layeredVarList[0]);
   }
@@ -2131,6 +2145,61 @@ lower::pft::getDependentVariableList(const semantics::Symbol &symbol) {
   return sda.getVariableList();
 }
 
+static bool
+isGenericHiddingProcedurePointer(const Fortran::semantics::Symbol &sym) {
+  if (const auto *generic = sym.detailsIf<Fortran::semantics::GenericDetails>())
+    if (const Fortran::semantics::Symbol *specific = generic->specific())
+      return Fortran::semantics::IsProcedurePointer(*specific);
+  return false;
+}
+
+/// Collect the canonical list of host [sub]module variables referenced in \p
+/// funit. A symbol is considered a host module variable if it is an
+/// ObjectEntityDetails, a ProcedurePointer, or a NamelistDetails symbol whose
+/// ultimate owning scope is a [sub]module scope containing \p funit's scope.
+/// Namelist groups are expanded to the set of their objects.
+static void collectHostAssociatedModuleVariables(
+    const Fortran::lower::pft::FunctionLikeUnit &funit,
+    llvm::SetVector<const Fortran::semantics::Symbol *> &moduleVariables) {
+  auto addIfHostModuleVariable = [&](const Fortran::semantics::Symbol &sym) {
+    const Fortran::semantics::Symbol &ultimate = sym.GetUltimate();
+    const auto *namelistDetails =
+        ultimate.detailsIf<Fortran::semantics::NamelistDetails>();
+    if (!ultimate.has<Fortran::semantics::ObjectEntityDetails>() &&
+        !Fortran::semantics::IsProcedurePointer(ultimate) &&
+        !isGenericHiddingProcedurePointer(ultimate) && !namelistDetails)
+      return;
+    const Fortran::semantics::Scope &symbolScope =
+        Fortran::semantics::FollowHostAssoc(sym).owner();
+    if (symbolScope.kind() != Fortran::semantics::Scope::Kind::Module)
+      return;
+    if (namelistDetails) {
+      // Namelist symbols are processed on the fly in IO lowering, which
+      // needs to be able to access each of their objects. Capture the
+      // objects rather than the namelist symbol itself.
+      for (const auto &namelistObject : namelistDetails->objects())
+        moduleVariables.insert(&namelistObject->GetUltimate());
+    } else {
+      moduleVariables.insert(&ultimate);
+    }
+  };
+  Fortran::lower::pft::visitAllSymbols(funit, addIfHostModuleVariable);
+}
+
+/// Create an ordered list of equivalences and variables from host
+/// [sub]modules of \p funit that are referenced in \p funit. The result is
+/// not cached.
+lower::pft::VariableList
+lower::pft::getHostModuleVariableList(const FunctionLikeUnit &funit) {
+  LLVM_DEBUG(llvm::dbgs() << "\ngetHostModuleVariableList of funit scope <"
+                          << &funit.getScope() << "> "
+                          << funit.getScope().GetName() << "\n");
+  llvm::SetVector<const semantics::Symbol *> moduleVariables;
+  collectHostAssociatedModuleVariables(funit, moduleVariables);
+  SymbolDependenceAnalysis sda(moduleVariables);
+  return sda.getVariableList();
+}
+
 namespace {
 /// Helper class to find all the symbols referenced in a FunctionLikeUnit.
 /// It defines a parse tree visitor doing a deep visit in all nodes with
diff --git a/flang/lib/Semantics/tools.cpp b/flang/lib/Semantics/tools.cpp
index e0e8727251ed8..86f0f59adc6f4 100644
--- a/flang/lib/Semantics/tools.cpp
+++ b/flang/lib/Semantics/tools.cpp
@@ -262,7 +262,7 @@ bool DoesScopeContain(const Scope *maybeAncestor, const Symbol &symbol) {
   return DoesScopeContain(maybeAncestor, symbol.owner());
 }
 
-static const Symbol &FollowHostAssoc(const Symbol &symbol) {
+const Symbol &FollowHostAssoc(const Symbol &symbol) {
   for (const Symbol *s{&symbol};;) {
     const auto *details{s->detailsIf<HostAssocDetails>()};
     if (!details) {
diff --git a/flang/test/Lower/OpenMP/target-map-complex.f90 b/flang/test/Lower/OpenMP/target-map-complex.f90
index fc01bdafe51ed..2325aec79e65b 100644
--- a/flang/test/Lower/OpenMP/target-map-complex.f90
+++ b/flang/test/Lower/OpenMP/target-map-complex.f90
@@ -8,8 +8,8 @@
 !CHECK-FPRIV: omp.private {type = firstprivate} @[[PRIV_32:.*]] : complex<f32> copy {
 
 !CHECK-LABEL: func.func @_QMmPbar()
-!CHECK:  %[[V0:[0-9]+]]:2 = hlfir.declare {{.*}} (!fir.ref<complex<f64>>) -> (!fir.ref<complex<f64>>, !fir.ref<complex<f64>>)
 !CHECK:  %[[V1:[0-9]+]]:2 = hlfir.declare {{.*}} (!fir.ref<complex<f32>>) -> (!fir.ref<complex<f32>>, !fir.ref<complex<f32>>)
+!CHECK:  %[[V0:[0-9]+]]:2 = hlfir.declare {{.*}} (!fir.ref<complex<f64>>) -> (!fir.ref<complex<f64>>, !fir.ref<complex<f64>>)
 !CHECK-FPRIV:  %[[V2:[0-9]+]] = omp.map.info var_ptr(%[[V1]]#0 : !fir.ref<complex<f32>>, complex<f32>) {{.*}} capture(ByCopy)
 !CHECK-FPRIV:  %[[V3:[0-9]+]] = omp.map.info var_ptr(%[[V0]]#0 : !fir.ref<complex<f64>>, complex<f64>) {{.*}} capture(ByRef)
 !CHECK-FPRIV:  omp.target map_entries(%[[V2]] -> {{.*}}, %[[V3]] -> {{.*}} : !fir.ref<complex<f32>>, !fir.ref<complex<f64>>) private(@[[PRIV_32]] %[[V1]]#0 -> %{{.*}} [map_idx=0], @[[PRIV_64]] %[[V0]]#0 -> %{{.*}} [map_idx=1] : !fir.ref<complex<f32>>, !fir.ref<complex<f64>>) {
diff --git a/flang/test/Lower/c-interoperability.f90 b/flang/test/Lower/c-interoperability.f90
index 724763027763a..73886f74c69ef 100644
--- a/flang/test/Lower/c-interoperability.f90
+++ b/flang/test/Lower/c-interoperability.f90
@@ -2,8 +2,6 @@
 
 ! CHECK-LABEL: func @_QMc_interoperability_testPget_a_thing() -> !fir.type<_QMc_interoperability_testTthing_with_pointer{cptr:!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>}> {
 ! CHECK:         %[[VAL_0:.*]] = fir.dummy_scope : !fir.dscope
-! CHECK:         %[[VAL_1:.*]] = fir.address_of(@_QM__fortran_builtinsEC__builtin_c_null_ptr) : !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>
-! CHECK:         %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_1]] {fortran_attrs = #fir.var_attrs<parameter>, uniq_name = "_QM__fortran_builtinsEC__builtin_c_null_ptr"} : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>) -> (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>)
 ! CHECK:         %[[VAL_3:.*]] = fir.address_of(@_QMc_interoperability_testEthis_thing) : !fir.ref<!fir.type<_QMc_interoperability_testTthing_with_pointer{cptr:!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>}>>
 ! CHECK:         %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_3]] {uniq_name = "_QMc_interoperability_testEthis_thing"} : (!fir.ref<!fir.type<_QMc_interoperability_testTthing_with_pointer{cptr:!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>}>>) -> (!fir.ref<!fir.type<_QMc_interoperability_testTthing_with_pointer{cptr:!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>}>>, !fir.ref<!fir.type<_QMc_interoperability_testTthing_with_pointer{cptr:!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>}>>)
 ! CHECK:         %[[VAL_5:.*]] = fir.alloca !fir.type<_QMc_interoperability_testTthing_with_pointer{cptr:!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>}> {bindc_name = "get_a_thing", uniq_name = "_QMc_interoperability_testFget_a_thingEget_a_thing"}
diff --git a/flang/test/Lower/host_module_variable_instantiation.f90 b/flang/test/Lower/host_module_variable_instantiation.f90
new file mode 100644
index 0000000000000..029b7b9329589
--- /dev/null
+++ b/flang/test/Lower/host_module_variable_instantiation.f90
@@ -0,0 +1,19 @@
+! Test that host module variables are not instantiated inside
+! module procedure when they are not needed.
+
+! RUN: %flang_fc1 -emit-hlfir %s -o - | FileCheck %s
+
+module test_host_module_instantiation
+  integer :: var1, var2
+contains
+subroutine foo()
+  call bar(var1)
+end subroutine
+end module
+
+! CHECK-LABEL:  func.func @_QMtest_host_module_instantiationPfoo(
+! CHECK-NOT: fir.address_of
+! CHECK: %[[ADDR1:.*]] = fir.address_of(@_QMtest_host_module_instantiationEvar1) : !fir.ref<i32>
+! CHECK: hlfir.declare %[[ADDR1]]
+! CHECK-NOT: fir.address_of
+! CHECK: return
diff --git a/flang/test/Lower/proc_pointer_hidden_by_generic.f90 b/flang/test/Lower/proc_pointer_hidden_by_generic.f90
new file mode 100644
index 0000000000000..f8efb027fd0d2
--- /dev/null
+++ b/flang/test/Lower/proc_pointer_hidden_by_generic.f90
@@ -0,0 +1,33 @@
+! Test instantiation of procedure pointer hidden behind
+! generic interface host associated from a module.
+
+! RUN: %flang_fc1 -emit-hlfir %s -o - | FileCheck %s
+
+module m_proc_pointer_hidden_by_generic
+procedure (proc2),pointer :: p
+interface p
+  procedure proc1, p
+end interface
+contains
+subroutine proc1(i)
+  integer :: i
+end subroutine
+subroutine proc2(x)
+  real :: x
+end subroutine
+
+subroutine test()
+p=>proc2
+end subroutine
+end
+
+! CHECK-LABEL:   func.func @_QMm_proc_pointer_hidden_by_genericPtest(
+! CHECK:           %[[DUMMY_SCOPE_0:.*]] = fir.dummy_scope : !fir.dscope
+! CHECK:           %[[ADDRESS_OF_0:.*]] = fir.address_of(@_QMm_proc_pointer_hidden_by_genericEp) : !fir.ref<!fir.boxproc<(!fir.ref<f32>) -> ()>>
+! CHECK:           %[[DECLARE_0:.*]]:2 = hlfir.declare %[[ADDRESS_OF_0]] {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QMm_proc_pointer_hidden_by_genericEp"} : (!fir.ref<!fir.boxproc<(!fir.ref<f32>) -> ()>>) -> (!fir.ref<!fir.boxproc<(!fir.ref<f32>) -> ()>>, !fir.ref<!fir.boxproc<(!fir.ref<f32>) -> ()>>)
+! CHECK:           %[[ADDRESS_OF_1:.*]] = fir.address_of(@_QMm_proc_pointer_hidden_by_genericPproc2) : (!fir.ref<f32>) -> ()
+! CHECK:           %[[EMBOXPROC_0:.*]] = fir.emboxproc %[[ADDRESS_OF_1]] : ((!fir.ref<f32>) -> ()) -> !fir.boxproc<() -> ()>
+! CHECK:           %[[CONVERT_0:.*]] = fir.convert %[[EMBOXPROC_0]] : (!fir.boxproc<() -> ()>) -> !fir.boxproc<(!fir.ref<f32>) -> ()>
+! CHECK:           fir.store %[[CONVERT_0]] to %[[DECLARE_0]]#0 : !fir.ref<!fir.boxproc<(!fir.ref<f32>) -> ()>>
+! CHECK:           return
+! CHECK:         }

``````````

</details>


https://github.com/llvm/llvm-project/pull/193689


More information about the flang-commits mailing list