[flang-commits] [flang] Reland [flang][debug] Emit debug info for named constants- #213974 (PR #215369)

[Abid Qadeer via flang-commits]

================
@@ -510,8 +521,27 @@ void AddDebugInfoPass::handleGlobalOp(fir::GlobalOp globalOp,
   mlir::OpBuilder builder(context);
 
   std::pair result = fir::NameUniquer::deconstruct(globalOp.getSymName());
-  if (result.first != fir::NameUniquer::NameKind::VARIABLE)
+  switch (result.first) {
+  case fir::NameUniquer::NameKind::VARIABLE:
+    break;
+  case fir::NameUniquer::NameKind::CONSTANT:
+    // A constant local to a procedure is described while walking that
+    // procedure, where `scope` is its DISubprogramAttr. Reaching here with any
+    // other scope means the procedure is not in the IR, typically because it
+    // was never called and got removed while its constant survived. There is
+    // no procedure to attach the constant to, and describing it at compile
+    // unit scope would wrongly make it visible everywhere.
+    if (!isModuleLevelName(result.second) &&
+        !mlir::isa<mlir::LLVM::DISubprogramAttr>(scope))
+      return;
+    // Don't describe a constant for which we only have a declaration. It could
+    // leave an unresolved symbol in the debug information.
+    if (!globalOp.isInitialized())
----------------
abidh wrote:

Thanks for passing this on, the analysis is right, and both symptoms reproduce. Here is what was happening and what I have changed.

**The mechanism.** `decl = !globalOp.isInitialized()` answers *"does this unit initialize this global"*, and it was being used to answer *"is this unit where the module is defined"*. Those coincide for a module variable but not for a named constant, and since the walk now reaches constants, the first constant to arrive decided the question. Because `DIModuleAttr` is immutable and the map is first-writer-wins, that decision then stuck for every other member of the module. Using an intrinsic module materializes some of its constants locally — `_QMiso_fortran_envECcharacter_kinds` comes out `linkonce_odr` with an initializer — so one of those was enough to make `iso_fortran_env` look locally defined.

**The fix, in two parts.**

1. *Only describe a named constant this unit defines.* A constant imported from a module arrives as an uninitialized `fir.global`, i.e. a declaration, and describing it produced a location pointing at a symbol nothing defines. This is what created the undefined references, and it is already in the branch you reviewed.
2. *Decide module definedness before any `DIModuleAttr` is built.* Lowering emits one
`fir.module_debug_imports` for every module and submodule it compiles, so that set is exactly the set of modules defined here. It is now collected once up front and every `DIModuleAttr` is built from the same answer. No individual global gets a vote, so visit order cannot change the result.

These should fix the relocation and order sensitivity issue as describe above.

**A note on submodules.** A submodule has no `DIModuleAttr` of its own today; the entities it defines
hang off its ancestor module's `DIModuleAttr`. So for the submodule's own constant `sq` to be describable at all, the ancestor must be a definition in that object, a declaration carries no scope, and anything under it cannot reach the compile unit and is dropped, which is precisely what happens to `kid2` above. The new pass therefore also treats an ancestor as defined when a submodule of it is compiled here, and that is what makes the two objects agree. I have kept that deliberately narrow, because it is a stopgap rather than a model of what submodules are.  Describing submodules properly, with a DIModuleAttr of their own is follow-up work.



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