[flang-commits] [flang] dc744d9 - [flang][debug] Describe submodules in the debug info (#225852)

via flang-commits flang-commits at lists.llvm.org
Fri Sep 25 03:05:21 PDT 2026


Author: Abid Qadeer
Date: 2026-09-25T11:05:14+01:00
New Revision: dc744d9839ca01185a9d19790f0b6bc18361d667

URL: https://github.com/llvm/llvm-project/commit/dc744d9839ca01185a9d19790f0b6bc18361d667
DIFF: https://github.com/llvm/llvm-project/commit/dc744d9839ca01185a9d19790f0b6bc18361d667.diff

LOG:  [flang][debug] Describe submodules in the debug info (#225852)

A submodule had no presence of its own in the debug info. Everything it
defined was attributed to the module at the root of its ancestry, so a
debugger showed one module per source file, every one of them carrying
the
name and the line of that ancestor, and it listed the variables of a
submodule as if the ancestor had declared them.

This PR describes a submodule in its own right. There are two
independent strands to it, and they meet only in that both decide which
module an entity is scoped in.

## Thread A: describing the submodule

### Recording the ancestry

A submodule is not a first class entity in FIR. It is not mangled into
the name of anything it declares in a way that distinguishes it from a
module, and its own name is unqualified, so nothing downstream could
tell `impl` of
`shapes` from a module called `impl`. Lowering now records the ancestry
on `fir.module_debug_imports`, which it already emits once for every
module and submodule it compiles:

```
fir.module_debug_imports "shapes" { }
fir.module_debug_imports "impl" in "shapes" parent "shapes" { }
fir.module_debug_imports "deep" in "shapes" parent "impl" { }
```

`ancestor_module` is the module at the root, and `parent_module` is the
one directly above, which is that same module for a first level
submodule and a submodule of it otherwise. Both are needed and neither
can be derived from
the other in a unit that compiles the submodule alone. The division
mirrors semantics, which keeps `ModuleDetails::ancestor()` and
`ModuleDetails::parent()` apart for the same reason.

### Naming

DWARF has no way to say that a module entry is a submodule, so the name
has to carry it. The submodule is named after the module at the root of
its ancestry and itself, joined with a `.`, which is what gfortran does:

```
DW_TAG_module  DW_AT_name ("shapes.impl")
DW_TAG_module  DW_AT_name ("shapes.deep")
```

The standard makes the pair unique among the submodules of a program,
and a `.` cannot appear in a Fortran identifier, so the result can never
be confused with a module that is really called that. A nested submodule
is named after the root and itself, not after the submodule containing
it, so `deep` of `shapes:impl` is `shapes.deep`. That too follows
gfortran, and it keeps the name independent of how deep the nesting
happens to be.

### Importing the parent

Scoping the entities of a submodule at the submodule loses something: a
submodule also has access to everything declared above it, and nothing
in the name of a module entry says so. Each submodule therefore imports
the one
directly above it:

```
DW_TAG_module  DW_AT_name ("shapes.impl")
  DW_TAG_imported_module  DW_AT_import ("shapes")
DW_TAG_module  DW_AT_name ("shapes.deep")
  DW_TAG_imported_module  DW_AT_import ("shapes.impl")
```

A debugger walks that chain up from wherever it is stopped, so an entity
declared anywhere above a submodule is still found by name. This is
again what gfortran emits, down to importing the immediate parent rather
than the root.

The entries hang off the compile unit rather than off any one procedure,
so that a submodule needs a single one however many procedures it holds.
That makes the compile unit name a module that names the compile unit
back, so it
is built as a recursive attribute, the same way a subprogram holding
imported entities already is. A unit that compiles no submodule is
unchanged.

## Thread B: separate module procedures

A separate module procedure is mangled with the module that declares its
interface rather than the submodule that defines it, because that is the
name its callers use:

```fortran
module shapes
  interface
    module function square(n) result(r)   ! declared here
```
```fortran
submodule (shapes) impl
contains
  module function square(n) result(r)     ! defined here
```
```
DW_AT_linkage_name ("_QMshapesPsquare")   ! names shapes, not impl
```

Every other entity of a submodule carries the whole chain in its uniqued
name: `_QMshapesSimplSdeepEdeep_var` names `shapes`, `impl` and `deep`
in turn. A separate module procedure names only the ancestor, so the
submodule defining it is recorded nowhere, and the procedure was
reported as belonging to the ancestor. Lowering now notes that submodule
on the function as `fir.defining_submodule`.

With that, nothing is scoped at the ancestor of a submodule any more,
and `AddDebugInfoPass::markSubmoduleAncestorsDefined` goes away. It
marked the ancestor as defined wherever a submodule was compiled, so
that the entities hanging off it could reach a compile unit; its comment
said it should go once submodules were described in their own right.

## What this looks like in a debugger

Given a module, a submodule defining its procedure, and a program
calling it:

```fortran
module subpar
  interface
    module subroutine hello()
    end subroutine
  end interface
end module subpar
```
```fortran
submodule (subpar) subkid
contains
  module subroutine hello()
    print *, 'hello from submodule'
  end subroutine hello
end submodule subkid
```

A breakpoint on the bare name works before and after, but it now reports
where
the procedure really is, and it can be qualified that way too:

| `gdb -batch -ex 'break ...'` | before | after |
| --- | --- | --- |
| `hello` | `in subpar::hello at kid.f90:4` | `in subpar.subkid::hello
at kid.f90:4` |
| `subpar.subkid::hello` | `Function ... not defined.` | `in
subpar.subkid::hello at kid.f90:4` |
| `subpar::hello` | `in subpar::hello at kid.f90:4` | `Function ... not
defined.` |

The last row is the one to note: `break subpar::hello` stops working,
since the procedure is no longer described as belonging to `subpar`
alone. That is a deliberate change rather than a step backwards and
gfortran behaves exactly as this patch does on all three spellings, so
this brings flang into line with it rather than away from it.

The import is what keeps name lookup working from inside a submodule.
Stopped in a procedure of `submodule (par3) kid3`, which uses `pv`, `pk`
and `ptbl` from `par3` and its own `sq`:

```
(gdb) print pv         $1 = 1
(gdb) print pk         $1 = 5
(gdb) print ptbl       $1 = (7, 8, 9)
(gdb) print sq         $1 = (9, 8, 7)
```

Fixes https://github.com/llvm/llvm-project/issues/215811

Assisted By: Cursor

---------

Co-authored-by: Cursor <cursoragent at cursor.com>

Added: 
    flang/test/Integration/debug-submodule-ancestor.F90
    flang/test/Integration/debug-submodule-nested.F90
    flang/test/Lower/debug-submodule.f90
    flang/test/Transforms/debug-submodule.fir

Modified: 
    flang/include/flang/Optimizer/Dialect/FIROps.td
    flang/include/flang/Optimizer/Dialect/FIROpsSupport.h
    flang/lib/Lower/Bridge.cpp
    flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
    flang/test/Fir/fir-ops.fir
    flang/test/Integration/debug-submodule-procedure.F90
    flang/test/Transforms/debug-module-submodule-procedure.fir

Removed: 
    


################################################################################
diff  --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td
index 0ab176cba2c8e..1af68c4d846f5 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -3093,13 +3093,23 @@ def fir_ModuleDebugImportsOp : fir_Op<"module_debug_imports", [
     The region holds one `fir.use_stmt` per `USE`, in program order. This
     operation has no runtime effect. It is emitted only when full debug
     information is requested.
+
+    A submodule is not a first class entity in FIR, and its name is not
+    qualified, so its ancestry is recorded here. `ancestor_module` names the
+    module at the root of it, which is also what tells a submodule and a module
+    apart. `parent_module` names the one directly above, which is that same
+    module for a first level submodule and a submodule of it otherwise.
+    Semantics keeps the two apart in the same way.
   }];
 
-  let arguments = (ins StrAttr:$module_name);
+  let arguments = (ins StrAttr:$module_name,
+                       OptionalAttr<StrAttr>:$ancestor_module,
+                       OptionalAttr<StrAttr>:$parent_module);
   let regions = (region SizedRegion<1>:$uses);
 
   let assemblyFormat = [{
-    $module_name attr-dict-with-keyword $uses
+    $module_name (`in` $ancestor_module^)? (`parent` $parent_module^)?
+    attr-dict-with-keyword $uses
   }];
 }
 

diff  --git a/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h b/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h
index 2051b36fb9b53..5d320c8634b5c 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h
+++ b/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h
@@ -130,6 +130,14 @@ static constexpr llvm::StringRef getHostSymbolAttrName() {
   return "fir.host_symbol";
 }
 
+/// Attribute naming the submodule that defines a separate module procedure.
+/// Such a procedure is mangled with the module that declares its interface, so
+/// this is the only record of where it is really defined. It is only set when
+/// full debug information is requested.
+static constexpr llvm::StringRef getDefiningSubmoduleAttrName() {
+  return "fir.defining_submodule";
+}
+
 /// Attribute containing the original name of a function from before the
 /// ExternalNameConverision pass runs
 static constexpr llvm::StringRef getInternalFuncNameAttrName() {

diff  --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 6d3331b164dca..c7558806ee241 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -310,9 +310,27 @@ emitModuleDebugImports(Fortran::lower::AbstractConverter &converter,
   mlir::OpBuilder::InsertionGuard guard(builder);
   builder.setInsertionPoint(mlirModule.getBody(), mlirModule.getBody()->end());
 
+  // A submodule is named after its ancestor module in the debug info, and has
+  // access to everything in the submodule directly above it, so record both.
+  mlir::StringAttr ancestorNameAttr;
+  mlir::StringAttr parentNameAttr;
+  if (const auto *details{
+          modSym->detailsIf<Fortran::semantics::ModuleDetails>()};
+      details && details->isSubmodule()) {
+    if (const Fortran::semantics::Scope *ancestor{details->ancestor()};
+        ancestor && ancestor->symbol())
+      ancestorNameAttr = mlir::StringAttr::get(
+          builder.getContext(), ancestor->symbol()->name().ToString());
+    if (const Fortran::semantics::Scope *parent{details->parent()};
+        parent && parent->symbol())
+      parentNameAttr = mlir::StringAttr::get(
+          builder.getContext(), parent->symbol()->name().ToString());
+  }
+
   auto op = fir::ModuleDebugImportsOp::create(
       builder, loc,
-      mlir::StringAttr::get(builder.getContext(), modSym->name().ToString()));
+      mlir::StringAttr::get(builder.getContext(), modSym->name().ToString()),
+      ancestorNameAttr, parentNameAttr);
   mlir::Region &region = op.getUses();
   mlir::Block *block = new mlir::Block();
   region.push_back(block);
@@ -333,6 +351,32 @@ emitUseStatementsFromFunit(Fortran::lower::AbstractConverter &converter,
     emitUseStmtOp(converter, builder, loc, preservedStmt, scope);
 }
 
+/// Record the submodule that defines a separate module procedure. Its name is
+/// mangled with the module that declares its interface, so the submodule would
+/// otherwise be lost and its debug info would point at the module instead.
+static void setDefiningSubmoduleForDebug(
+    Fortran::lower::AbstractConverter &converter, mlir::func::FuncOp func,
+    const Fortran::lower::pft::FunctionLikeUnit &funit) {
+  // This option is set whenever more than line tables are asked for, which is
+  // also when a module is described, so it stands in for full debug info here.
+  if (!converter.getLoweringOptions().getPreserveUseDebugInfo())
+    return;
+
+  const Fortran::semantics::Scope &scope = funit.getScope();
+  if (!scope.parent().IsSubmodule())
+    return;
+  const Fortran::semantics::Symbol *sym = scope.symbol();
+  if (!sym || !sym->attrs().test(Fortran::semantics::Attr::MODULE))
+    return;
+  const Fortran::semantics::Symbol *submodule = scope.parent().symbol();
+  if (!submodule)
+    return;
+
+  func->setAttr(
+      fir::getDefiningSubmoduleAttrName(),
+      mlir::StringAttr::get(func.getContext(), submodule->name().ToString()));
+}
+
 /// Helper class to generate the runtime type info global data and the
 /// fir.type_info operations that contain the dipatch tables (if any).
 /// The type info global data is required to describe the derived type to the
@@ -6298,6 +6342,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
 
     // Emit USE statement operations for debug info generation
     emitUseStatementsFromFunit(*this, *builder, toLocation(), funit);
+    setDefiningSubmoduleForDebug(*this, func, funit);
 
     // Map host associated symbols from parent procedure if any.
     if (funit.parentHasHostAssoc())

diff  --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index 36365dc5daf79..a334a16859f4c 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -83,10 +83,16 @@ class AddDebugInfoPass : public fir::impl::AddDebugInfoBase<AddDebugInfoPass> {
   /// Names of the modules whose DIModule this compilation unit defines.
   llvm::StringSet<> definedModuleNames;
 
+  mlir::LLVM::DIModuleAttr createModuleAttr(const std::string &name,
+                                            mlir::LLVM::DIFileAttr fileAttr,
+                                            mlir::LLVM::DIScopeAttr scope);
   mlir::LLVM::DIModuleAttr
   getOrCreateModuleAttr(const std::string &name,
                         mlir::LLVM::DIFileAttr fileAttr,
                         mlir::LLVM::DIScopeAttr scope);
+  llvm::SmallVector<mlir::LLVM::DINodeAttr>
+  buildSubmoduleImports(mlir::ModuleOp module, mlir::LLVM::DIFileAttr fileAttr,
+                        mlir::LLVM::DIScopeAttr cuAttr);
   mlir::LLVM::DICommonBlockAttr
   getOrCreateCommonBlockAttr(llvm::StringRef name,
                              mlir::LLVM::DIFileAttr fileAttr,
@@ -117,7 +123,6 @@ class AddDebugInfoPass : public fir::impl::AddDebugInfoBase<AddDebugInfoPass> {
       llvm::SetVector<mlir::LLVM::DIImportedEntityAttr> &importedEntities);
   void buildModuleDebugImportsMap(mlir::ModuleOp module);
   void buildDefinedModuleNames(mlir::ModuleOp module);
-  void markSubmoduleAncestorsDefined(mlir::ModuleOp module);
   void expandUseStmtForDebug(
       fir::UseStmtOp useOp, mlir::LLVM::DISubprogramAttr spAttr,
       mlir::LLVM::DIFileAttr fileAttr, mlir::LLVM::DICompileUnitAttr cuAttr,
@@ -148,6 +153,28 @@ class AddDebugInfoPass : public fir::impl::AddDebugInfoBase<AddDebugInfoPass> {
                            fir::cg::XDeclareOp typeGenDeclOp);
 };
 
+// A submodule has no name of its own in the debug info. Like gfortran, name it
+// after its ancestor module and itself, which the standard guarantees to be
+// enough to tell the submodules of a program apart. Join the two with a '.', as
+// gfortran does. A '.' cannot appear in a Fortran identifier, so the result can
+// never collide with a module that is really called that, which matters because
+// a module is looked up by this name alone.
+std::string getDebugModuleName(llvm::StringRef ancestor, llvm::StringRef name) {
+  if (ancestor.empty())
+    return name.str();
+  return (ancestor + "." + name).str();
+}
+
+// The (sub)module that owns an entity, as named in the debug info. A uniqued
+// name records the whole ancestry, from the module down to the innermost
+// submodule, so the first and the last of those are what we need.
+std::string getDebugModuleName(llvm::ArrayRef<std::string> modules) {
+  assert(!modules.empty() && "not a module entity");
+  if (modules.size() == 1)
+    return modules.front();
+  return getDebugModuleName(modules.front(), modules.back());
+}
+
 /// Whether \p loc already carries debug information of type \c AttrT, fused
 /// onto it by this pass. A location is fused for unrelated reasons too, most
 /// notably one that came from an INCLUDE'd file, so what the fusion holds has
@@ -506,38 +533,42 @@ mlir::LLVM::DICommonBlockAttr AddDebugInfoPass::getOrCreateCommonBlockAttr(
 // The `module` does not have a first class representation in the `FIR`. We
 // extract information about it from the name of the identifiers and keep a
 // map to avoid duplication.
+mlir::LLVM::DIModuleAttr
+AddDebugInfoPass::createModuleAttr(const std::string &name,
+                                   mlir::LLVM::DIFileAttr fileAttr,
+                                   mlir::LLVM::DIScopeAttr scope) {
+  mlir::MLIRContext *context = &getContext();
+  unsigned line = 0;
+  bool decl = !definedModuleNames.contains(name);
+
+  // The location of the fir.module_debug_imports is that of the MODULE
+  // statement. A module that has none is not defined here, and gets no line.
+  if (auto iter{moduleDebugImportsByName.find(name)};
+      iter != moduleDebugImportsByName.end()) {
+    line = fir::getLineFromLoc(iter->second.getLoc());
+    fileAttr = fir::getFileAttrFromLoc(iter->second.getLoc(), fileAttr);
+  }
+
+  // When decl is true, it means that module is only being used in this
+  // compilation unit and it is defined elsewhere. But if the file/line/scope
+  // fields are valid, the module is not merged with its definition and is
+  // considered 
diff erent. So we only set those fields when decl is false.
+  return mlir::LLVM::DIModuleAttr::get(
+      context, decl ? nullptr : fileAttr, decl ? nullptr : scope,
+      mlir::StringAttr::get(context, name),
+      /* configMacros */ mlir::StringAttr(),
+      /* includePath */ mlir::StringAttr(),
+      /* apinotes */ mlir::StringAttr(), decl ? 0 : line, decl);
+}
+
 mlir::LLVM::DIModuleAttr
 AddDebugInfoPass::getOrCreateModuleAttr(const std::string &name,
                                         mlir::LLVM::DIFileAttr fileAttr,
                                         mlir::LLVM::DIScopeAttr scope) {
-  mlir::MLIRContext *context = &getContext();
-  mlir::LLVM::DIModuleAttr modAttr;
-  if (auto iter{moduleMap.find(name)}; iter != moduleMap.end()) {
-    modAttr = iter->getValue();
-  } else {
-    unsigned line = 0;
-    bool decl = !definedModuleNames.contains(name);
-
-    // The location of the fir.module_debug_imports is that of the MODULE
-    // statement. A module that has none is not defined here, and gets no line.
-    if (auto iter{moduleDebugImportsByName.find(name)};
-        iter != moduleDebugImportsByName.end()) {
-      line = fir::getLineFromLoc(iter->second.getLoc());
-      fileAttr = fir::getFileAttrFromLoc(iter->second.getLoc(), fileAttr);
-    }
-
-    // When decl is true, it means that module is only being used in this
-    // compilation unit and it is defined elsewhere. But if the file/line/scope
-    // fields are valid, the module is not merged with its definition and is
-    // considered 
diff erent. So we only set those fields when decl is false.
-    modAttr = mlir::LLVM::DIModuleAttr::get(
-        context, decl ? nullptr : fileAttr, decl ? nullptr : scope,
-        mlir::StringAttr::get(context, name),
-        /* configMacros */ mlir::StringAttr(),
-        /* includePath */ mlir::StringAttr(),
-        /* apinotes */ mlir::StringAttr(), decl ? 0 : line, decl);
-    moduleMap[name] = modAttr;
-  }
+  if (auto iter{moduleMap.find(name)}; iter != moduleMap.end())
+    return iter->getValue();
+  mlir::LLVM::DIModuleAttr modAttr = createModuleAttr(name, fileAttr, scope);
+  moduleMap[name] = modAttr;
   return modAttr;
 }
 
@@ -568,7 +599,8 @@ AddDebugInfoPass::getModuleAttrFromGlobalOp(fir::GlobalOp globalOp,
   if (sp)
     scope = sp.getCompileUnit();
 
-  return getOrCreateModuleAttr(result.second.modules[0], fileAttr, scope);
+  return getOrCreateModuleAttr(getDebugModuleName(result.second.modules),
+                               fileAttr, scope);
 }
 
 void AddDebugInfoPass::handleGlobalOp(fir::GlobalOp globalOp,
@@ -775,7 +807,16 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
       }
     }
   } else if (!result.second.modules.empty()) {
-    Scope = getOrCreateModuleAttr(result.second.modules[0], fileAttr, cuAttr);
+    // A separate module procedure is mangled with the module that declares its
+    // interface, so its own name does not name the submodule that defines it.
+    // The root of the ancestry is the same either way.
+    auto submodule = funcOp->getAttrOfType<mlir::StringAttr>(
+        fir::getDefiningSubmoduleAttrName());
+    std::string name = submodule
+                           ? getDebugModuleName(result.second.modules.front(),
+                                                submodule.getValue())
+                           : getDebugModuleName(result.second.modules);
+    Scope = getOrCreateModuleAttr(name, fileAttr, cuAttr);
   }
 
   auto addTargetOpDISP = [&](llvm::ArrayRef<mlir::Attribute> entities) {
@@ -1036,7 +1077,8 @@ void AddDebugInfoPass::handleUseStatements(
 void AddDebugInfoPass::buildModuleDebugImportsMap(mlir::ModuleOp module) {
   moduleDebugImportsByName.clear();
   module.walk([&](fir::ModuleDebugImportsOp op) {
-    moduleDebugImportsByName[op.getModuleName().str()] = op;
+    moduleDebugImportsByName[getDebugModuleName(
+        op.getAncestorModule().value_or(""), op.getModuleName())] = op;
   });
 }
 
@@ -1053,47 +1095,53 @@ void AddDebugInfoPass::buildDefinedModuleNames(mlir::ModuleOp module) {
   definedModuleNames.clear();
   for (auto &entry : moduleDebugImportsByName)
     definedModuleNames.insert(entry.getKey());
-  markSubmoduleAncestorsDefined(module);
 }
 
-// We do not describe submodules yet: a submodule gets no DIModuleAttr of its
-// own and the entities it defines hang off the DIModuleAttr of its ancestor
-// module. So the ancestor has to be a definition in a unit that compiles the
-// submodule. Were it a declaration, it would carry no scope, and those entities
-// would not be able to reach a compile unit and would be dropped from the debug
-// information entirely. Members of a submodule name it in their mangled name,
-// so take the ancestor from them. All of this goes away once submodules are
-// described in their own right.
-void AddDebugInfoPass::markSubmoduleAncestorsDefined(mlir::ModuleOp module) {
-  // The mangled name of a module level global carries its whole module chain,
-  // so mark the ancestor as defined whenever a submodule below it is compiled
-  // here.
-  for (auto globalOp : module.getOps<fir::GlobalOp>()) {
-    std::pair result = fir::NameUniquer::deconstruct(globalOp.getSymName());
-    if (!isModuleLevelName(result.second))
-      continue;
-    llvm::ArrayRef<std::string> modules = result.second.modules;
-    for (const std::string &submodule : modules.drop_front()) {
-      if (moduleDebugImportsByName.contains(submodule)) {
-        definedModuleNames.insert(modules.front());
-        break;
-      }
-    }
-  }
-
-  // Handle a submodule whose members are all procedures, which has no global to
-  // go by. A procedure without a body is defined elsewhere and says nothing
-  // about what this unit defines.
-  for (auto funcOp : module.getOps<mlir::func::FuncOp>()) {
-    if (funcOp.isExternal())
-      continue;
-    mlir::Attribute attr = funcOp->getAttr(fir::getInternalFuncNameAttrName());
-    llvm::StringRef name =
-        attr ? mlir::cast<mlir::StringAttr>(attr).getValue() : funcOp.getName();
-    std::pair result = fir::NameUniquer::deconstruct(name);
-    if (!result.second.modules.empty())
-      definedModuleNames.insert(result.second.modules.front());
-  }
+// A submodule has access to everything in the submodule directly above it, and
+// through that one to the whole of its ancestry, but the name of its
+// DW_TAG_module says nothing about it. Import the parent into each submodule,
+// as gfortran does, so that a debugger stopped in a procedure of a submodule
+// can still find the entities declared above it. The imports of a nested
+// submodule form a chain that it walks up to the module at the root.
+//
+// These belong to the compile unit rather than to any one procedure, so that a
+// submodule needs a single one however many procedures it holds. That makes the
+// compile unit refer to a module that refers back to it, which is what the
+// recursive attribute the caller builds is for.
+//
+// The modules named here are built directly rather than taken from the cache,
+// because they name the placeholder unit while every other use of them names
+// the real one, and only the latter should be handed out. Everything else about
+// them is settled before either is built, so the two describe the same module
+// and become one once the unit they name is resolved.
+llvm::SmallVector<mlir::LLVM::DINodeAttr>
+AddDebugInfoPass::buildSubmoduleImports(mlir::ModuleOp module,
+                                        mlir::LLVM::DIFileAttr fileAttr,
+                                        mlir::LLVM::DIScopeAttr cuAttr) {
+  mlir::MLIRContext *context = &getContext();
+  llvm::SmallVector<mlir::LLVM::DINodeAttr> imports;
+  // Walked rather than taken from the map so that the order is that of the
+  // program and does not depend on how the names happen to hash.
+  module.walk([&](fir::ModuleDebugImportsOp op) {
+    std::optional<llvm::StringRef> ancestor = op.getAncestorModule();
+    std::optional<llvm::StringRef> parent = op.getParentModule();
+    if (!ancestor || !parent)
+      return;
+    // The parent of a first level submodule is the root module, whose name
+    // stands alone. Any other parent is a submodule, and is named like one.
+    std::string parentName = *parent == *ancestor
+                                 ? parent->str()
+                                 : getDebugModuleName(*ancestor, *parent);
+    std::string name = getDebugModuleName(*ancestor, op.getModuleName());
+    imports.push_back(mlir::LLVM::DIImportedEntityAttr::get(
+        context, llvm::dwarf::DW_TAG_imported_module,
+        createModuleAttr(name, fileAttr, cuAttr),
+        createModuleAttr(parentName, fileAttr, cuAttr),
+        fir::getFileAttrFromLoc(op.getLoc(), fileAttr),
+        fir::getLineFromLoc(op.getLoc()), /*name=*/mlir::StringAttr(),
+        /*elements=*/{}));
+  });
+  return imports;
 }
 
 void AddDebugInfoPass::expandUseStmtForDebug(
@@ -1212,12 +1260,28 @@ void AddDebugInfoPass::runOnOperation() {
       debugLevel == mlir::LLVM::DIEmissionKind::DebugDirectivesOnly
           ? mlir::LLVM::DINameTableKind::None
           : mlir::LLVM::DINameTableKind::Default;
+  // Each submodule compiled here imports the one above it, and those entries
+  // hang off the compile unit. They are scoped at modules that name the unit
+  // they are part of, so build them against a placeholder of it first, and give
+  // the unit the matching recursive id once it holds them.
+  mlir::DistinctAttr cuRecId =
+      mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
+  llvm::SmallVector<mlir::LLVM::DINodeAttr> cuImports = buildSubmoduleImports(
+      module, fileAttr,
+      mlir::cast<mlir::LLVM::DICompileUnitAttr>(
+          mlir::LLVM::DICompileUnitAttr::getRecSelf(cuRecId)));
+
   mlir::LLVM::DICompileUnitAttr cuAttr = mlir::LLVM::DICompileUnitAttr::get(
       mlir::DistinctAttr::create(mlir::UnitAttr::get(context)),
       llvm::dwarf::getLanguage("DW_LANG_Fortran95"), fileAttr, producer,
       isOptimized, debugLevel, debugInfoForProfiling, nameTableKind,
       splitDwarfFile.empty() ? mlir::StringAttr()
-                             : mlir::StringAttr::get(context, splitDwarfFile));
+                             : mlir::StringAttr::get(context, splitDwarfFile),
+      cuImports);
+  // A unit that compiles no submodule holds no import, and stays as it was.
+  if (!cuImports.empty())
+    cuAttr =
+        mlir::cast<mlir::LLVM::DICompileUnitAttr>(cuAttr.withRecId(cuRecId));
 
   // Process module globals early.
   // Walk through all DeclareOps in functions and process globals that are

diff  --git a/flang/test/Fir/fir-ops.fir b/flang/test/Fir/fir-ops.fir
index 7cf9caef13160..97032b61cf666 100644
--- a/flang/test/Fir/fir-ops.fir
+++ b/flang/test/Fir/fir-ops.fir
@@ -1124,3 +1124,14 @@ func.func @test_create_box_2d(%arg0: !fir.ref<!fir.array<?x?xf32>>, %lb0: index,
 fir.module_debug_imports "debug_mod" {
   fir.use_stmt "used_mod"
 }
+
+// A submodule records the module at the root of its ancestry and its parent.
+// CHECK-LABEL: fir.module_debug_imports "debug_sub" in "debug_mod" parent "debug_mod"
+// CHECK-NEXT: }
+fir.module_debug_imports "debug_sub" in "debug_mod" parent "debug_mod" {
+}
+
+// CHECK-LABEL: fir.module_debug_imports "debug_deep" in "debug_mod" parent "debug_sub"
+// CHECK-NEXT: }
+fir.module_debug_imports "debug_deep" in "debug_mod" parent "debug_sub" {
+}

diff  --git a/flang/test/Integration/debug-submodule-ancestor.F90 b/flang/test/Integration/debug-submodule-ancestor.F90
new file mode 100644
index 0000000000000..cd08310be5c8e
--- /dev/null
+++ b/flang/test/Integration/debug-submodule-ancestor.F90
@@ -0,0 +1,32 @@
+! RUN: rm -rf %t && mkdir -p %t
+! RUN: %flang_fc1 -fsyntax-only -DSTEP=1 -J%t %s
+! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone -J%t %s -o - \
+! RUN:   | FileCheck %s
+
+! Test that compiling a submodule on its own leaves the ancestor a declaration,
+! so that it still merges with the unit that defines it.
+
+#if STEP == 1
+module anc_shapes
+  implicit none
+  integer :: mod_var = 1
+  interface
+    module subroutine hello()
+    end subroutine
+  end interface
+end module anc_shapes
+#else
+submodule (anc_shapes) impl
+contains
+  module subroutine hello()
+  end subroutine hello
+end submodule impl
+
+subroutine standalone()
+  use anc_shapes
+  mod_var = 2
+end subroutine standalone
+#endif
+
+! CHECK-DAG: !DIModule(scope: ![[#]], name: "anc_shapes.impl", file: ![[#]], line: 19)
+! CHECK-DAG: !DIModule(scope: null, name: "anc_shapes", isDecl: true)

diff  --git a/flang/test/Integration/debug-submodule-nested.F90 b/flang/test/Integration/debug-submodule-nested.F90
new file mode 100644
index 0000000000000..9eceb74320f97
--- /dev/null
+++ b/flang/test/Integration/debug-submodule-nested.F90
@@ -0,0 +1,40 @@
+! RUN: rm -rf %t && mkdir -p %t
+! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone -J%t %s -o - \
+! RUN:   | FileCheck %s
+
+! Test the import chain of a nested submodule: it imports the submodule that
+! contains it, which imports the module at the root.
+
+module nested_shapes
+  implicit none
+  integer :: root_var = 1
+  interface
+    module subroutine go()
+    end subroutine
+  end interface
+end module nested_shapes
+
+submodule (nested_shapes) mid
+  integer :: mid_var = 2
+end submodule mid
+
+submodule (nested_shapes:mid) leaf
+  integer :: leaf_var = 3
+contains
+  module subroutine go()
+    print *, root_var, mid_var, leaf_var
+  end subroutine go
+end submodule leaf
+
+! CHECK-DAG: ![[ROOT:[0-9]+]] = !DIModule(scope: ![[#]], name: "nested_shapes"
+! CHECK-DAG: ![[MID:[0-9]+]] = !DIModule(scope: ![[#]], name: "nested_shapes.mid"
+! CHECK-DAG: ![[LEAF:[0-9]+]] = !DIModule(scope: ![[#]], name: "nested_shapes.leaf"
+
+! CHECK-DAG: !DIImportedEntity(tag: DW_TAG_imported_module, scope: ![[MID]], entity: ![[ROOT]]
+! CHECK-DAG: !DIImportedEntity(tag: DW_TAG_imported_module, scope: ![[LEAF]], entity: ![[MID]]
+
+! An entity stays in the submodule that declares it.
+! CHECK-DAG: !DIGlobalVariable(name: "root_var", {{.*}}scope: ![[ROOT]],
+! CHECK-DAG: !DIGlobalVariable(name: "mid_var", {{.*}}scope: ![[MID]],
+! CHECK-DAG: !DIGlobalVariable(name: "leaf_var", {{.*}}scope: ![[LEAF]],
+! CHECK-DAG: !DISubprogram(name: "go", {{.*}}scope: ![[LEAF]],

diff  --git a/flang/test/Integration/debug-submodule-procedure.F90 b/flang/test/Integration/debug-submodule-procedure.F90
index d8c8fea8a6cf4..9a758e901bab3 100644
--- a/flang/test/Integration/debug-submodule-procedure.F90
+++ b/flang/test/Integration/debug-submodule-procedure.F90
@@ -20,5 +20,9 @@ end subroutine hello
 end submodule subkid
 #endif
 
-! CHECK: !DISubprogram(name: "hello", linkageName: "_QMsubparPhello", scope: ![[MOD:[0-9]+]]
-! CHECK: ![[MOD]] = !DIModule(scope: ![[#]], name: "subpar"
+! CHECK-DAG: ![[MOD:[0-9]+]] = !DIModule(scope: ![[#]], name: "subpar.subkid"
+! CHECK-DAG: !DISubprogram(name: "hello", linkageName: "_QMsubparPhello", scope: ![[MOD]]
+
+! The submodule imports its ancestor.
+! CHECK-DAG: ![[ANC:[0-9]+]] = !DIModule(scope: null, name: "subpar", isDecl: true)
+! CHECK-DAG: !DIImportedEntity(tag: DW_TAG_imported_module, scope: ![[MOD]], entity: ![[ANC]]

diff  --git a/flang/test/Lower/debug-submodule.f90 b/flang/test/Lower/debug-submodule.f90
new file mode 100644
index 0000000000000..9c3881495e1df
--- /dev/null
+++ b/flang/test/Lower/debug-submodule.f90
@@ -0,0 +1,47 @@
+! RUN: %flang_fc1 -emit-fir -debug-info-kind=standalone %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir %s -o - | FileCheck %s --check-prefix=NO_DEBUG
+! RUN: %flang_fc1 -emit-fir -debug-info-kind=line-tables-only %s -o - | FileCheck %s --check-prefix=NO_DEBUG
+
+! Test that lowering records the ancestry of a submodule, and the submodule
+! that defines a separate module procedure, only when debug info asks for it.
+
+! NO_DEBUG-NOT: fir.module_debug_imports
+! NO_DEBUG-NOT: fir.defining_submodule
+
+! Only a separate module procedure needs the attribute. Any other procedure
+! has the submodule in its own name already.
+! CHECK-DAG: func.func @_QMshapesPsquare({{.*}}attributes {fir.defining_submodule = "impl"}
+! CHECK-DAG: func.func @_QMshapesSimplSdeepPdeep_helper() {
+
+! The parent of a first level submodule is the module at the root, and that of
+! a nested one is the submodule containing it.
+! CHECK-DAG: fir.module_debug_imports "shapes" {
+! CHECK-DAG: fir.module_debug_imports "impl" in "shapes" parent "shapes" {
+! CHECK-DAG: fir.module_debug_imports "deep" in "shapes" parent "impl" {
+
+module shapes
+  interface
+    module function square(n) result(r)
+      integer, intent(in) :: n
+      integer :: r
+    end function square
+  end interface
+end module shapes
+
+submodule (shapes) impl
+  integer :: sub_var = 7
+contains
+  module function square(n) result(r)
+    integer, intent(in) :: n
+    integer :: r
+    r = n * n
+  end function square
+end submodule impl
+
+submodule (shapes:impl) deep
+  integer :: deep_var = 11
+contains
+  subroutine deep_helper()
+    deep_var = deep_var + sub_var
+  end subroutine deep_helper
+end submodule deep

diff  --git a/flang/test/Transforms/debug-module-submodule-procedure.fir b/flang/test/Transforms/debug-module-submodule-procedure.fir
index db054a913e3a4..60ec503009f65 100644
--- a/flang/test/Transforms/debug-module-submodule-procedure.fir
+++ b/flang/test/Transforms/debug-module-submodule-procedure.fir
@@ -1,10 +1,14 @@
 // RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s
 
+// Test a unit that compiles a submodule alone. Only that submodule is defined
+// here; the ancestor it imports and a module that is merely used are both
+// declarations.
+
 module {
   // Marks `subkid` as compiled here, as lowering does for every submodule.
-  fir.module_debug_imports "subkid" {
+  fir.module_debug_imports "subkid" in "subpar" parent "subpar" {
   } loc(#loc0)
-  func.func @_QMsubparPhello() {
+  func.func @_QMsubparPhello() attributes {fir.defining_submodule = "subkid"} {
     return
   } loc(#loc1)
   func.func private @_QMelsewherePthere() loc(#loc2)
@@ -13,8 +17,14 @@ module {
 #loc1 = loc("kid.f90":3:3)
 #loc2 = loc("kid.f90":6:3)
 
-// CHECK-DAG: #[[CU:.*]] = #llvm.di_compile_unit<{{.*}}>
-// CHECK-DAG: #[[SUBPAR:.*]] = #llvm.di_module<{{.*}}scope = #[[CU]], name = "subpar"{{.*}}>
+// CHECK-DAG: #[[CU_SELF:.*]] = #llvm.di_compile_unit<recId = [[RECID:.+]], isRecSelf = true>
+// CHECK-DAG: #[[CU:.*]] = #llvm.di_compile_unit<recId = [[RECID]], id = {{.*}}>
+// CHECK-DAG: #[[SUBPAR:.*]] = #llvm.di_module<name = "subpar", isDecl = true>
 // CHECK-DAG: #[[ELSEWHERE:.*]] = #llvm.di_module<name = "elsewhere", isDecl = true>
-// CHECK-DAG: #llvm.di_subprogram<{{.*}}scope = #[[SUBPAR]], name = "hello", linkageName = "_QMsubparPhello"{{.*}}>
+
+// CHECK-DAG: #[[SUBKID:.*]] = #llvm.di_module<{{.*}}scope = #[[CU]], name = "subpar.subkid"{{.*}}>
+// CHECK-DAG: #llvm.di_subprogram<{{.*}}scope = #[[SUBKID]], name = "hello", linkageName = "_QMsubparPhello"{{.*}}>
 // CHECK-DAG: #llvm.di_subprogram<{{.*}}scope = #[[ELSEWHERE]], name = "there", linkageName = "_QMelsewherePthere"{{.*}}>
+
+// CHECK-DAG: #[[SUBKID_S:.*]] = #llvm.di_module<{{.*}}scope = #[[CU_SELF]], name = "subpar.subkid"{{.*}}>
+// CHECK-DAG: #llvm.di_imported_entity<tag = DW_TAG_imported_module, scope = #[[SUBKID_S]], entity = #[[SUBPAR]]

diff  --git a/flang/test/Transforms/debug-submodule.fir b/flang/test/Transforms/debug-submodule.fir
new file mode 100644
index 0000000000000..638eecdfa63f3
--- /dev/null
+++ b/flang/test/Transforms/debug-submodule.fir
@@ -0,0 +1,72 @@
+// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s
+
+// Test how a submodule is described: its name, where it is described, what is
+// scoped in it, and the module it imports.
+
+module {
+  fir.module_debug_imports "shapes" {
+  } loc(#loc_shapes)
+  fir.global @_QMshapesEparent_var : i32 {
+    %0 = fir.zero_bits i32
+    fir.has_value %0 : i32
+  } loc(#loc_parent_var)
+
+  fir.module_debug_imports "impl" in "shapes" parent "shapes" {
+  } loc(#loc_impl)
+  fir.global @_QMshapesSimplEsub_var : i32 {
+    %0 = fir.zero_bits i32
+    fir.has_value %0 : i32
+  } loc(#loc_sub_var)
+  func.func @_QMshapesPsquare() attributes {fir.defining_submodule = "impl"} {
+    return
+  } loc(#loc_square)
+
+  fir.module_debug_imports "deep" in "shapes" parent "impl" {
+  } loc(#loc_deep)
+  fir.global @_QMshapesSimplSdeepEdeep_var : i32 {
+    %0 = fir.zero_bits i32
+    fir.has_value %0 : i32
+  } loc(#loc_deep_var)
+  func.func @_QMshapesSimplSdeepPdeep_helper() {
+    return
+  } loc(#loc_helper)
+}
+#loc_shapes = loc("shapes.f90":5:1)
+#loc_parent_var = loc("shapes.f90":7:14)
+#loc_impl = loc("impl.f90":6:1)
+#loc_sub_var = loc("impl.f90":8:14)
+#loc_square = loc("impl.f90":10:3)
+#loc_deep = loc("deep.f90":12:1)
+#loc_deep_var = loc("deep.f90":14:14)
+#loc_helper = loc("deep.f90":16:3)
+
+// Each module is described in the file that defines it.
+// CHECK-DAG: #[[F_SHAPES:.+]] = #llvm.di_file<"shapes.f90" in {{.*}}>
+// CHECK-DAG: #[[F_IMPL:.+]] = #llvm.di_file<"impl.f90" in {{.*}}>
+// CHECK-DAG: #[[F_DEEP:.+]] = #llvm.di_file<"deep.f90" in {{.*}}>
+
+// The compile unit is built as a placeholder and then holding the imports, so
+// a module that carries one is checked against each form.
+// CHECK-DAG: #[[CU_SELF:.+]] = #llvm.di_compile_unit<recId = [[RECID:.+]], isRecSelf = true>
+// CHECK-DAG: #[[CU:.+]] = #llvm.di_compile_unit<recId = [[RECID]], id = {{.*}}>
+
+// A submodule is named after the module at the root of its ancestry, nested or
+// not.
+// CHECK-DAG: #[[SHAPES:.+]] = #llvm.di_module<file = #[[F_SHAPES]], scope = #[[CU]], name = "shapes", line = 5>
+// CHECK-DAG: #[[IMPL:.+]] = #llvm.di_module<file = #[[F_IMPL]], scope = #[[CU]], name = "shapes.impl", line = 6>
+// CHECK-DAG: #[[DEEP:.+]] = #llvm.di_module<file = #[[F_DEEP]], scope = #[[CU]], name = "shapes.deep", line = 12>
+
+// Each submodule imports the one directly above it.
+// CHECK-DAG: #[[SHAPES_S:.+]] = #llvm.di_module<file = #[[F_SHAPES]], scope = #[[CU_SELF]], name = "shapes", line = 5>
+// CHECK-DAG: #[[IMPL_S:.+]] = #llvm.di_module<file = #[[F_IMPL]], scope = #[[CU_SELF]], name = "shapes.impl", line = 6>
+// CHECK-DAG: #[[DEEP_S:.+]] = #llvm.di_module<file = #[[F_DEEP]], scope = #[[CU_SELF]], name = "shapes.deep", line = 12>
+// CHECK-DAG: #llvm.di_imported_entity<tag = DW_TAG_imported_module, scope = #[[IMPL_S]], entity = #[[SHAPES_S]]
+// CHECK-DAG: #llvm.di_imported_entity<tag = DW_TAG_imported_module, scope = #[[DEEP_S]], entity = #[[IMPL_S]]
+
+// An entity is scoped in the submodule that declares it. A separate module
+// procedure goes by its attribute, anything else by its uniqued name.
+// CHECK-DAG: #llvm.di_global_variable<scope = #[[SHAPES]], name = "parent_var"
+// CHECK-DAG: #llvm.di_global_variable<scope = #[[IMPL]], name = "sub_var"
+// CHECK-DAG: #llvm.di_global_variable<scope = #[[DEEP]], name = "deep_var"
+// CHECK-DAG: #llvm.di_subprogram<{{.*}}scope = #[[IMPL]], name = "square"
+// CHECK-DAG: #llvm.di_subprogram<{{.*}}scope = #[[DEEP]], name = "deep_helper"


        


More information about the flang-commits mailing list