[flang][debug] Handle 'used' module. (llvm#107626)

abidh · web-flow · commit db64e69fa250 · 2024-09-11T09:31:53.000+01:00
As described in llvm#98883, we have to qualify a module variable name in debugger to get its value. This PR tries to remove this limitation. LLVM provides `DIImportedEntity` to handle such cases but the PR is made more complicated due to the following 2 issues. 1. The MLIR attributes are readonly and we have a circular dependency here. This has to be handled using the recursive interface provided by the MLIR. This requires us to first create a place holder `DISubprogramAttr` which is used in creating `DIImportedEntityAttr`. Later another `DISubprogramAttr` is created which replaces the place holder. 2. The flang IR does not provide any information about the 'used' module so this has to be extracted by doing a pass over the `DeclareOp` in the function. This presents certain limitation as 'only' and module variable renaming may not be handled properly. Due to the change in `DISubprogramAttr`, some tests also needed to be adjusted. Fixes llvm#98883.
diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -72,6 +72,10 @@ class AddDebugInfoPass : public fir::impl::AddDebugInfoBase<AddDebugInfoPass> {
                     mlir::LLVM::DICompileUnitAttr cuAttr,
                     fir::DebugTypeGenerator &typeGen,
                     mlir::SymbolTable *symbolTable);
+  std::optional<mlir::LLVM::DIModuleAttr>
+  getModuleAttrFromGlobalOp(fir::GlobalOp globalOp,
+                            mlir::LLVM::DIFileAttr fileAttr,
+                            mlir::LLVM::DIScopeAttr scope);
 };
 
 bool debugInfoIsAlreadySet(mlir::Location loc) {
@@ -152,6 +156,45 @@ mlir::LLVM::DIModuleAttr AddDebugInfoPass::getOrCreateModuleAttr(
   return modAttr;
 }
 
+/// If globalOp represents a module variable, return a ModuleAttr that
+/// represents that module.
+std::optional<mlir::LLVM::DIModuleAttr>
+AddDebugInfoPass::getModuleAttrFromGlobalOp(fir::GlobalOp globalOp,
+                                            mlir::LLVM::DIFileAttr fileAttr,
+                                            mlir::LLVM::DIScopeAttr scope) {
+  mlir::MLIRContext *context = &getContext();
+  mlir::OpBuilder builder(context);
+
+  std::pair result = fir::NameUniquer::deconstruct(globalOp.getSymName());
+  // Only look for module if this variable is not part of a function.
+  if (!result.second.procs.empty() || result.second.modules.empty())
+    return std::nullopt;
+
+  // DWARF5 says following about the fortran modules:
+  // A Fortran 90 module may also be represented by a module entry
+  // (but no declaration attribute is warranted because Fortran has no concept
+  // of a corresponding module body).
+  // But in practice, compilers use declaration attribute with a module in cases
+  // where module was defined in another source file (only being used in this
+  // one). The isInitialized() seems to provide the right information
+  // but inverted. It is true where module is actually defined but false where
+  // it is used.
+  // FIXME: Currently we don't have the line number on which a module was
+  // declared. We are using a best guess of line - 1 where line is the source
+  // line of the first member of the module that we encounter.
+  unsigned line = getLineFromLoc(globalOp.getLoc());
+
+  mlir::LLVM::DISubprogramAttr sp =
+      mlir::dyn_cast_if_present<mlir::LLVM::DISubprogramAttr>(scope);
+  // Modules are generated at compile unit scope
+  if (sp)
+    scope = sp.getCompileUnit();
+
+  return getOrCreateModuleAttr(result.second.modules[0], fileAttr, scope,
+                               std::max(line - 1, (unsigned)1),
+                               !globalOp.isInitialized());
+}
+
 void AddDebugInfoPass::handleGlobalOp(fir::GlobalOp globalOp,
                                       mlir::LLVM::DIFileAttr fileAttr,
                                       mlir::LLVM::DIScopeAttr scope,
@@ -174,33 +217,11 @@ void AddDebugInfoPass::handleGlobalOp(fir::GlobalOp globalOp,
     return;
 
   unsigned line = getLineFromLoc(globalOp.getLoc());
+  std::optional<mlir::LLVM::DIModuleAttr> modOpt =
+      getModuleAttrFromGlobalOp(globalOp, fileAttr, scope);
+  if (modOpt)
+    scope = *modOpt;
 
-  // DWARF5 says following about the fortran modules:
-  // A Fortran 90 module may also be represented by a module entry
-  // (but no declaration attribute is warranted because Fortran has no concept
-  // of a corresponding module body).
-  // But in practice, compilers use declaration attribute with a module in cases
-  // where module was defined in another source file (only being used in this
-  // one). The isInitialized() seems to provide the right information
-  // but inverted. It is true where module is actually defined but false where
-  // it is used.
-  // FIXME: Currently we don't have the line number on which a module was
-  // declared. We are using a best guess of line - 1 where line is the source
-  // line of the first member of the module that we encounter.
-
-  if (result.second.procs.empty()) {
-    // Only look for module if this variable is not part of a function.
-    if (result.second.modules.empty())
-      return;
-
-    // Modules are generated at compile unit scope
-    if (mlir::LLVM::DISubprogramAttr sp =
-            mlir::dyn_cast_if_present<mlir::LLVM::DISubprogramAttr>(scope))
-      scope = sp.getCompileUnit();
-
-    scope = getOrCreateModuleAttr(result.second.modules[0], fileAttr, scope,
-                                  line - 1, !globalOp.isInitialized());
-  }
   mlir::LLVM::DITypeAttr diType =
       typeGen.convertType(globalOp.getType(), fileAttr, scope, declOp);
   auto gvAttr = mlir::LLVM::DIGlobalVariableAttr::get(
@@ -262,15 +283,18 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
       mlir::LLVM::DIFileAttr::get(context, fileName, filePath);
 
   // Only definitions need a distinct identifier and a compilation unit.
-  mlir::DistinctAttr id;
+  mlir::DistinctAttr id, id2;
   mlir::LLVM::DIScopeAttr Scope = fileAttr;
   mlir::LLVM::DICompileUnitAttr compilationUnit;
   mlir::LLVM::DISubprogramFlags subprogramFlags =
       mlir::LLVM::DISubprogramFlags{};
   if (isOptimized)
     subprogramFlags = mlir::LLVM::DISubprogramFlags::Optimized;
   if (!funcOp.isExternal()) {
+    // Place holder and final function have to have different IDs, otherwise
+    // translation code will reject one of them.
     id = mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
+    id2 = mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
     compilationUnit = cuAttr;
     subprogramFlags =
         subprogramFlags | mlir::LLVM::DISubprogramFlags::Definition;
@@ -299,14 +323,69 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
                                   line - 1, false);
   }
 
-  auto spAttr = mlir::LLVM::DISubprogramAttr::get(
-      context, id, compilationUnit, Scope, funcName, fullName, funcFileAttr,
-      line, line, subprogramFlags, subTypeAttr, /*retainedNodes=*/{});
-  funcOp->setLoc(builder.getFusedLoc({funcOp->getLoc()}, spAttr));
-
   // Don't process variables if user asked for line tables only.
-  if (debugLevel == mlir::LLVM::DIEmissionKind::LineTablesOnly)
+  if (debugLevel == mlir::LLVM::DIEmissionKind::LineTablesOnly) {
+    auto spAttr = mlir::LLVM::DISubprogramAttr::get(
+        context, id, compilationUnit, Scope, funcName, fullName, funcFileAttr,
+        line, line, subprogramFlags, subTypeAttr, /*retainedNodes=*/{});
+    funcOp->setLoc(builder.getFusedLoc({l}, spAttr));
     return;
+  }
+
+  mlir::DistinctAttr recId =
+      mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
+
+  // The debug attribute in MLIR are readonly once created. But in case of
+  // imported entities, we have a circular dependency. The
+  // DIImportedEntityAttr requires scope information (DISubprogramAttr in this
+  // case) and DISubprogramAttr requires the list of imported entities. The
+  // MLIR provides a way where a DISubprogramAttr an be created with a certain
+  // recID and be used in places like DIImportedEntityAttr. After that another
+  // DISubprogramAttr can be created with same recID but with list of entities
+  // now available. The MLIR translation code takes care of updating the
+  // references. Note that references will be updated only in the things that
+  // are part of DISubprogramAttr (like DIImportedEntityAttr) so we have to
+  // create the final DISubprogramAttr before we process local variables.
+  // Look at DIRecursiveTypeAttrInterface for more details.
+
+  auto spAttr = mlir::LLVM::DISubprogramAttr::get(
+      context, recId, /*isRecSelf=*/true, id, compilationUnit, Scope, funcName,
+      fullName, funcFileAttr, line, line, subprogramFlags, subTypeAttr,
+      /*retainedNodes=*/{});
+
+  // There is no direct information in the IR for any 'use' statement in the
+  // function. We have to extract that information from the DeclareOp. We do
+  // a pass on the DeclareOp and generate ModuleAttr and corresponding
+  // DIImportedEntityAttr for that module.
+  // FIXME: As we are depending on the variables to see which module is being
+  // 'used' in the function, there are certain limitations.
+  // For things like 'use mod1, only: v1', whole module will be brought into the
+  // namespace in the debug info. It is not a problem as such unless there is a
+  // clash of names.
+  // There is no information about module variable renaming
+  llvm::DenseSet<mlir::LLVM::DIImportedEntityAttr> importedModules;
+  funcOp.walk([&](fir::cg::XDeclareOp declOp) {
+    if (&funcOp.front() == declOp->getBlock())
+      if (auto global =
+              symbolTable->lookup<fir::GlobalOp>(declOp.getUniqName())) {
+        std::optional<mlir::LLVM::DIModuleAttr> modOpt =
+            getModuleAttrFromGlobalOp(global, fileAttr, cuAttr);
+        if (modOpt) {
+          auto importedEntity = mlir::LLVM::DIImportedEntityAttr::get(
+              context, llvm::dwarf::DW_TAG_imported_module, spAttr, *modOpt,
+              fileAttr, /*line=*/1, /*name=*/nullptr, /*elements*/ {});
+          importedModules.insert(importedEntity);
+        }
+      }
+  });
+  llvm::SmallVector<mlir::LLVM::DINodeAttr> entities(importedModules.begin(),
+                                                     importedModules.end());
+  // We have the imported entities now. Generate the final DISubprogramAttr.
+  spAttr = mlir::LLVM::DISubprogramAttr::get(
+      context, recId, /*isRecSelf=*/false, id2, compilationUnit, Scope,
+      funcName, fullName, funcFileAttr, line, line, subprogramFlags,
+      subTypeAttr, entities);
+  funcOp->setLoc(builder.getFusedLoc({l}, spAttr));
 
   funcOp.walk([&](fir::cg::XDeclareOp declOp) {
     // FIXME: We currently dont handle variables that are not in the entry
diff --git a/flang/test/Integration/debug-module-2.f90 b/flang/test/Integration/debug-module-2.f90
@@ -17,7 +17,7 @@ module helper
   integer gli
 
   contains
-!CHECK-DAG: !DISubprogram(name: "test", linkageName: "_QMhelperPtest", scope: ![[MOD]], file: ![[FILE2]], line: [[@LINE+1]]{{.*}}unit: ![[CU]])
+!CHECK-DAG: !DISubprogram(name: "test", linkageName: "_QMhelperPtest", scope: ![[MOD]], file: ![[FILE2]], line: [[@LINE+1]]{{.*}}unit: ![[CU]]{{.*}})
     subroutine test()
     glr = 12.34
     gli = 67
diff --git a/flang/test/Transforms/debug-90683.fir b/flang/test/Transforms/debug-90683.fir
@@ -22,4 +22,4 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
 
 // CHECK-DAG: #[[TY:.*]] = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "real", sizeInBits = 64, encoding = DW_ATE_float>
 // CHECK-DAG: #[[TY1:.*]] = #llvm.di_subroutine_type<callingConvention = DW_CC_normal, types = #[[TY]], #[[TY]], #[[TY]]>
-// CHECK-DAG: #{{.*}} = #llvm.di_subprogram<scope = #{{.*}}, name = "cabs", linkageName = "cabs", file = #{{.*}}, line = {{.*}}, scopeLine = {{.*}}, type = #[[TY1]]>
+// CHECK-DAG: #{{.*}} = #llvm.di_subprogram<{{.*}}name = "cabs", linkageName = "cabs"{{.*}}, type = #[[TY1]]>
diff --git a/flang/test/Transforms/debug-fn-info.fir b/flang/test/Transforms/debug-fn-info.fir
@@ -69,7 +69,7 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
 // CHECK: #[[TY2:.*]] = #llvm.di_subroutine_type<callingConvention = DW_CC_normal, types = #[[INT4]], #[[INT8]], #[[REAL4]], #[[LOG4]]>
 
 // Line numbers should match the number in corresponding loc entry.
-// CHECK: #llvm.di_subprogram<id = {{.*}}, compileUnit = {{.*}}, scope = {{.*}}, name = "_QQmain", linkageName = "_QQmain", file = {{.*}}, line = 15, scopeLine = 15, subprogramFlags = Definition, type = #[[TY0]]>
-// CHECK: #llvm.di_subprogram<id = {{.*}}, compileUnit = {{.*}}, scope = {{.*}}, name = "fn1", linkageName = "_QFPfn1", file = {{.*}}, line = 26, scopeLine = 26, subprogramFlags = Definition, type = #[[TY1]]>
-// CHECK: #llvm.di_subprogram<id = {{.*}}, compileUnit = {{.*}}, scope = {{.*}}, name = "fn2", linkageName = "_QFPfn2", file = {{.*}}, line = 43, scopeLine = 43, subprogramFlags = Definition, type = #[[TY2]]>
+// CHECK: #llvm.di_subprogram<{{.*}}name = "_QQmain", linkageName = "_QQmain", file = {{.*}}, line = 15, scopeLine = 15, subprogramFlags = Definition, type = #[[TY0]]>
+// CHECK: #llvm.di_subprogram<{{.*}}name = "fn1", linkageName = "_QFPfn1", file = {{.*}}, line = 26, scopeLine = 26, subprogramFlags = Definition, type = #[[TY1]]>
+// CHECK: #llvm.di_subprogram<{{.*}}name = "fn2", linkageName = "_QFPfn2", file = {{.*}}, line = 43, scopeLine = 43, subprogramFlags = Definition, type = #[[TY2]]>
 
diff --git a/flang/test/Transforms/debug-imported-entity.fir b/flang/test/Transforms/debug-imported-entity.fir
@@ -0,0 +1,30 @@
+// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s
+
+
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
+  fir.global @_QMfooEv1 : i32 {
+    %0 = fir.zero_bits i32
+    fir.has_value %0 : i32
+  }
+  fir.global internal @_QFtestExyz : i32 {
+    %c12_i32 = arith.constant 12 : i32
+    fir.has_value %c12_i32 : i32
+  } loc(#loc4)
+  func.func @test() attributes {fir.bindc_name = "test"} {
+    %0 = fir.address_of(@_QMfooEv1) : !fir.ref<i32>
+    %1 = fircg.ext_declare %0 {uniq_name = "_QMfooEv1"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc1)
+    %4 = fir.address_of(@_QFtestExyz) : !fir.ref<i32>
+    %5 = fircg.ext_declare %4 {uniq_name = "_QFtestExyz"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc4)
+    return
+  } loc(#loc3)
+}
+#loc1 = loc("test.f90":2:14)
+#loc2 = loc("test.f90":6:1)
+#loc3 = loc("test.f90":10:1)
+#loc4 = loc("test.f90":13:1)
+
+// CHECK: #[[MOD:.+]] = #llvm.di_module<{{.*}}name = "foo"{{.*}}>
+// CHECK: #[[SP_REC:.+]] = #llvm.di_subprogram<recId = distinct[[[REC_ID:[0-9]+]]]<>, isRecSelf = true{{.*}}>
+// CHECK: #[[IMP_ENTITY:.+]] = #llvm.di_imported_entity<tag = DW_TAG_imported_module, scope = #[[SP_REC]], entity = #[[MOD]]{{.*}}>
+// CHECK: #[[SP:.+]] = #llvm.di_subprogram<recId = distinct[[[REC_ID]]]<>{{.*}}retainedNodes = #[[IMP_ENTITY]]>
+// CHECK: #llvm.di_global_variable<scope = #[[SP]], name = "xyz"{{.*}}>
diff --git a/flang/test/Transforms/debug-line-table-inc-file.fir b/flang/test/Transforms/debug-line-table-inc-file.fir
@@ -31,7 +31,7 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
 // CHECK: #[[LOC_INC_FILE:.*]] = loc("{{.*}}inc.f90":1:1)
 // CHECK: #[[LOC_FILE:.*]] = loc("{{.*}}simple.f90":3:1)
 // CHECK: #[[DI_CU:.*]] = #llvm.di_compile_unit<id = distinct[{{.*}}]<>, sourceLanguage = DW_LANG_Fortran95, file = #[[DI_FILE]], producer = "{{.*}}flang{{.*}}", isOptimized = false, emissionKind = LineTablesOnly>
-// CHECK: #[[DI_SP_INC:.*]] = #llvm.di_subprogram<id = distinct[{{.*}}]<>, compileUnit = #[[DI_CU]], scope = #[[DI_FILE]], name = "sinc", linkageName = "_QPsinc", file = #[[DI_INC_FILE]], {{.*}}>
-// CHECK: #[[DI_SP:.*]] = #llvm.di_subprogram<id = distinct[{{.*}}]<>, compileUnit = #[[DI_CU]], scope = #[[DI_FILE]], name = "_QQmain", linkageName = "_QQmain", file = #[[DI_FILE]], {{.*}}>
+// CHECK: #[[DI_SP_INC:.*]] = #llvm.di_subprogram<{{.*}}id = distinct[{{.*}}]<>, compileUnit = #[[DI_CU]], scope = #[[DI_FILE]], name = "sinc", linkageName = "_QPsinc", file = #[[DI_INC_FILE]], {{.*}}>
+// CHECK: #[[DI_SP:.*]] = #llvm.di_subprogram<{{.*}}id = distinct[{{.*}}]<>, compileUnit = #[[DI_CU]], scope = #[[DI_FILE]], name = "_QQmain", linkageName = "_QQmain", file = #[[DI_FILE]], {{.*}}>
 // CHECK: #[[FUSED_LOC_INC_FILE]] = loc(fused<#[[DI_SP_INC]]>[#[[LOC_INC_FILE]]])
 // CHECK: #[[FUSED_LOC_FILE]] = loc(fused<#[[DI_SP]]>[#[[LOC_FILE]]])
diff --git a/flang/test/Transforms/debug-local-global-storage-1.fir b/flang/test/Transforms/debug-local-global-storage-1.fir
@@ -45,7 +45,7 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<i64, dense<64> :
 // CHECK-DAG: #[[CU:.*]] = #llvm.di_compile_unit<{{.*}}>
 // CHECK-DAG: #[[MOD:.*]] = #llvm.di_module<{{.*}}scope = #[[CU]]{{.*}}name = "example"{{.*}}>
 // CHECK-DAG: #[[SP:.*]] = #llvm.di_subprogram<{{.*}}name = "_QQmain"{{.*}}>
-// CHECK-DAG: #[[MOD_SP:.*]] = #llvm.di_subprogram<{{.*}}name = "mod_sub"{{.*}}>
+// CHECK-DAG: #[[MOD_SP:.*]] = #llvm.di_subprogram<{{.*}}name = "mod_sub"{{.*}}retainedNodes = {{.*}}>
 // CHECK-DAG: #llvm.di_global_variable<scope = #[[SP]], name = "arr"{{.*}}line = 22{{.*}}>
 // CHECK-DAG: #llvm.di_global_variable<scope = #[[SP]], name = "s"{{.*}}line = 23{{.*}}>
 // CHECK-DAG: #llvm.di_global_variable<scope = #[[MOD_SP]], name = "ss"{{.*}}line = 12{{.*}}>
