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

[Eugene Epshteyn 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())
----------------
eugeneepshteyn wrote:

AI reviewer flagged the following here. This is beyond my current knowledge of debug info, so please check if this makes sense to you:

In `getModuleAttrFromGlobalOp` (`flang/lib/Optimizer/Transforms/AddDebugInfo.cpp`),
`decl = !globalOp.isInitialized()` answers *"does this unit initialize this global?"* and
is used to answer *"is this unit where the module is defined?"*. Those coincide for a
module **variable**, which is initialized exactly where its module is defined. They do not
coincide for a named **constant**: an intrinsic module's constants are materialized
locally (as `linkonce_odr`) in every using unit. Since the early walk now uses
`isModuleDataObject` instead of `isModuleVariable`, constants reach
`getOrCreateModuleAttr`, whose map is first-writer-wins — so one locally-materialized
constant makes the whole intrinsic module look locally defined.

Same 10-line source, `-g -O0`, patched vs. unpatched:

```fortran
program p
  use iso_fortran_env
  implicit none
  print *, 'hello'
end program p
```

| build | `DW_TAG_module "iso_fortran_env"` | `.rela.debug_info` relocations naming it |
|---|---|---|
| main (before this PR) | `DW_AT_declaration (true)` | 0 |
| **this PR** | `DW_AT_decl_file ("p01.f90") DW_AT_decl_line (75)` | **1** |

So the emitted DWARF states that `iso_fortran_env` is defined at line 75 of a 10-line user
program. The relocation is `R_X86_64_64 _QMiso_fortran_envECcharacter_kinds` against a
`WEAK DEFAULT` **defined** symbol, which is why nothing fails to link — but the statement
is still false, and it is a regression in metadata quality against main.

The same mechanism is **order-sensitive in ordinary user code**. Two submodule sources
differing only in the order of items in one `print`:

par3.f90:
```fortran
module par3
  implicit none
  integer :: pv = 1
  integer, parameter :: pk = 5
  integer, parameter :: ptbl(3) = [7,8,9]
  interface
    module subroutine work()
    end subroutine
  end interface
end module par3
```

kid3.f90:
```fortran
submodule (par3) kid3
  implicit none
  integer, parameter :: sq(3) = [9,8,7]
contains
  module subroutine work()
    print *, pv, pk, ptbl, sq
  end subroutine work
end submodule kid3
```

kid2.f90 (same as kid3, except the `print` order is different):
```fortran
submodule (par3) kid3
  implicit none
  integer, parameter :: sq(3) = [9,8,7]
contains
  module subroutine work()
    print *, sq, pv, pk, ptbl
  end subroutine work
end submodule kid3
```
| object | `.rela.debug_info` relocations naming `_QMpar3` |
|---|---|
| `kid3.o` | **0** |
| `kid4.o` | **2**, including `R_X86_64_64 _QMpar3Epv` — and `_QMpar3Epv` is `UND` in that object |

`kid4` links and runs correctly (`par3.o` defines `_QMpar3Epv`), so this is not a link
failure — but debug-info content that depends on the order of items in a `print` statement
is a fragile place to be, and it is the same predicate that caused the revert.


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