[flang-commits] [flang] [flang] handle indirect module variable use in internal procedure (PR #65324)

[via flang-commits]

https://github.com/jeanPerier created https://github.com/llvm/llvm-project/pull/65324:

When a module variable is referenced inside an internal procedure, but the use statement for the module is inside the host, semantics may not create any symbols with HostAssocDetails directly under the internal procedure scope.
So pft::getScopeVariableList, that is called in the bridge when lowering the internal procedure scope, failed to instantiate the module variables. This lead to "symbol is not mapped to any IR value" compile time errors.

This patch fixes the issue by adding the variables to the list of "captured" global variables from the host program, so that they are instantiated as part of the `internalProcedureBindings` in the bridge.

The rational of doing it that way instead of changing `getScopeVariableList` is that `getScopeVariableList` would have to import all the module variables used inside the host since it cannot know which ones are referenced inside the internal procedure from the semantics::Scope information. The fix in this patch only instantiates the module variables from the host that are actually referenced inside the internal procedure.

>From 086ebbbb0d941f49814a3f6f3e9830a010050baf Mon Sep 17 00:00:00 2001
From: Jean Perier <jperier at nvidia.com>
Date: Tue, 5 Sep 2023 03:16:14 -0700
Subject: [PATCH] [flang] handle indirect module variable use in internal
 procedure

When a module variable is referenced inside an internal procedure,
but the use statement for the module is inside the host, semantics
may not create any symbols with HostAssocDetails directly under the
internal procedure scope.
So pft::getScopeVariableList, that is called in the bridge when lowering
the internal procedure scope, failed to instantiate the module variables.
This lead to "symbol is not mapped to any IR value" compile time errors.

This patch fixes the issue by adding the variables to the list of "captured"
global variables from the host program, so that they are instantiated as
part of the `internalProcedureBindings` in the bridge.

The rational of doing it that way instead of changing
getScopeVariableList is that getScopeVariableList would have to import
all the module variables used inside the host since it cannot know which
ones are used inside the internal procedure from the Scope information.
The fix in this patch only instantiates the module variables from the host that
are actually referenced inside the internal procedure.
---
 flang/lib/Lower/Bridge.cpp                    | 23 +++++++++++----
 .../Lower/HLFIR/internal-procedures-2.f90     | 29 +++++++++++++++++++
 .../Lower/explicit-interface-results-2.f90    |  2 +-
 3 files changed, 48 insertions(+), 6 deletions(-)
 create mode 100644 flang/test/Lower/HLFIR/internal-procedures-2.f90

diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index d5de3679cffca30..e024d569a6e504f 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -383,6 +383,19 @@ class FirConverter : public Fortran::lower::AbstractConverter {
       declareFunction(f);
   }
 
+  /// Get the scope that is defining or using \p sym. The returned scope is not
+  /// the ultimate scope, since this helper does not traverse use association.
+  /// This allows capturing module variables that are referenced in an internal
+  /// procedure but whose use statement is inside the host program.
+  const Fortran::semantics::Scope &
+  getSymbolHostScope(const Fortran::semantics::Symbol &sym) {
+    const Fortran::semantics::Symbol *hostSymbol = &sym;
+    while (const auto *details =
+               hostSymbol->detailsIf<Fortran::semantics::HostAssocDetails>())
+      hostSymbol = &details->symbol();
+    return hostSymbol->owner();
+  }
+
   /// Collects the canonical list of all host associated symbols. These bindings
   /// must be aggregated into a tuple which can then be added to each of the
   /// internal procedure declarations and passed at each call site.
@@ -399,12 +412,12 @@ class FirConverter : public Fortran::lower::AbstractConverter {
       if (ultimate.has<Fortran::semantics::ObjectEntityDetails>() ||
           Fortran::semantics::IsProcedurePointer(ultimate) ||
           Fortran::semantics::IsDummy(sym) || namelistDetails) {
-        const Fortran::semantics::Scope &ultimateScope = ultimate.owner();
-        if (ultimateScope.kind() ==
+        const Fortran::semantics::Scope &symbolScope = getSymbolHostScope(sym);
+        if (symbolScope.kind() ==
                 Fortran::semantics::Scope::Kind::MainProgram ||
-            ultimateScope.kind() == Fortran::semantics::Scope::Kind::Subprogram)
-          if (ultimateScope != *internalScope &&
-              ultimateScope.Contains(*internalScope)) {
+            symbolScope.kind() == Fortran::semantics::Scope::Kind::Subprogram)
+          if (symbolScope != *internalScope &&
+              symbolScope.Contains(*internalScope)) {
             if (namelistDetails) {
               // So far, namelist symbols are processed on the fly in IO and
               // the related namelist data structure is not added to the symbol
diff --git a/flang/test/Lower/HLFIR/internal-procedures-2.f90 b/flang/test/Lower/HLFIR/internal-procedures-2.f90
new file mode 100644
index 000000000000000..bb05545bef1aa27
--- /dev/null
+++ b/flang/test/Lower/HLFIR/internal-procedures-2.f90
@@ -0,0 +1,29 @@
+! Test instantiation of module variables inside an internal subprogram
+! where the use statement is inside the host program.
+! RUN: bbc -emit-hlfir -o - %s | FileCheck %s
+
+module module_used_by_host
+ implicit none
+ integer :: indexed_by_var(2)
+ integer :: ref_in_implied_do
+ integer :: ref_in_forall(2)
+end module
+
+subroutine host_procedure
+ use module_used_by_host
+ implicit none
+contains
+ subroutine internal_procedure(i, mask)
+  integer :: i
+  logical :: mask(2)
+  indexed_by_var(i) = 0
+  print *, (/(ref_in_implied_do, integer::j=1,10)/)
+  forall (integer::k = 1:2)
+    ref_in_forall(k) = 0
+  end forall
+ end subroutine
+end subroutine
+! CHECK-LABEL: func.func @_QFhost_procedurePinternal_procedure(
+! CHECK:    fir.address_of(@_QMmodule_used_by_hostEindexed_by_var) : !fir.ref<!fir.array<2xi32>>
+! CHECK:    fir.address_of(@_QMmodule_used_by_hostEref_in_forall) : !fir.ref<!fir.array<2xi32>>
+! CHECK:    fir.address_of(@_QMmodule_used_by_hostEref_in_implied_do) : !fir.ref<i32>
diff --git a/flang/test/Lower/explicit-interface-results-2.f90 b/flang/test/Lower/explicit-interface-results-2.f90
index 06fa5e6ed480b0a..f77eb7157c237e7 100644
--- a/flang/test/Lower/explicit-interface-results-2.f90
+++ b/flang/test/Lower/explicit-interface-results-2.f90
@@ -94,7 +94,7 @@ subroutine host5()
   implicit none
   call internal_proc_a()
 contains
-! CHECK-LABEL: func @_QFhost5Pinternal_proc_a() {
+! CHECK-LABEL: func @_QFhost5Pinternal_proc_a() attributes {fir.internal_proc} {
   subroutine internal_proc_a()
     call takes_array(return_array())
 ! CHECK:  %[[VAL_0:.*]] = fir.address_of(@_QMsome_moduleEn_module) : !fir.ref<i32>