[flang-commits] [flang] [flang][debug] Use DICompileUnit as scope. (PR #96345)

[Abid Qadeer via flang-commits]

https://github.com/abidh created https://github.com/llvm/llvm-project/pull/96345

This PR is required for #96316 . For assumed size arrays, we don't have the size of the array in the last dimension. This is represented in metadata by having a DISubrange with no entry for count or upper bound.

LLVM verifier allows a `DISubrange` with no count or upper bound only for Fortran [here](https://github.com/llvm/llvm-project/blob/af478c82a5f6e42ff5d1e67dcefbf9dd0fee501c/llvm/lib/IR/Verifier.cpp#L1159). This language is set [here](https://github.com/llvm/llvm-project/blob/af478c82a5f6e42ff5d1e67dcefbf9dd0fee501c/llvm/lib/IR/Verifier.cpp#L1415) when DICompileUnit is processed. If a type is processed before `DICompileUnit` then Verifier will not allow `DISubrange` without count or upper bound entry. It will fail with error `Subrange must contain count or upperBound`.

Above scenario easily happens if a subroutine at global scope takes assumed size parameter. The [order of operands](https://github.com/llvm/llvm-project/blob/bf3e3289d67cb0fe136b0660cac39c24c9f65069/llvm/lib/IR/DebugInfoMetadata.cpp#L1140) in `DISubprogram` is such that `Type` is processed before `Unit` so we hit `Verifier::visitDISubrange` before language has been set to Fortran.

I have worked around this issue by setting the scope of such functions to be `DICompileUnit` instead of `DIFile`. As `Scope` metadata in `DISubprogram` is processed before `Type` so this problem goes away.

Some other observations about this change:
1. It does not make any difference to the generated DWARF even with an include file.
2. It bring subroutine inline with module as modules already set their scope to `DICompileUnit`.
3. I also looked at the classic flang compiler and it also uses `DICompileUnit` as scope for such subroutines probably for the same reasons.

>From 2fc3de9749ca8b4b72aff609979f5af88dea30d9 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Fri, 21 Jun 2024 14:55:27 +0100
Subject: [PATCH] [flang][debug] Use DICompileUnit as scope.

This PR is required for #96316. For assumed size arrays, we don't have
the size of the array in the last dimension. This is represented in
metadata by having a DISubrange with no entry for count or upper bound.

LLVM verifier allows a DISubrange with no count or upper bound only for
Fortran. This language is set when DICompileUnit is processed. If a type
is processed before DICompileUnit then Verifier will not allow
DISubrange with not count or upper bound entry. It will fail with error
"Subrange must contain count or upperBound".

Above scenario easily happens if a subroutine at global scope takes
assumed size parameter. The metadata on DISubprogram is such that type
is processed before unit so we hit Verifier::visitDISubrange before
language has been set to Fortran.

I have worked around this issue by setting the scope of such functions
to be DICompileUnit instead of DIFile. As scope metadata in
DISubprogram is processed before type so this problem does not
happen.

Some other observations about this change:
1. It does not make any difference to the generated DWARF.
2. It bring subroutine inline with module as modules already set their
scope to DiCompileUnit.
3. I also looked at the classic flang compiler and it also uses
DICompileUnit as scope for such subroutines probably for the same
reasons.
---
 flang/lib/Optimizer/Transforms/AddDebugInfo.cpp     | 2 +-
 flang/test/Transforms/debug-line-table-inc-file.fir | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index 10c71d3fc9551..2216157f3378d 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -266,7 +266,7 @@ void AddDebugInfoPass::runOnOperation() {
 
     // Only definitions need a distinct identifier and a compilation unit.
     mlir::DistinctAttr id;
-    mlir::LLVM::DIScopeAttr Scope = fileAttr;
+    mlir::LLVM::DIScopeAttr Scope = cuAttr;
     mlir::LLVM::DICompileUnitAttr compilationUnit;
     mlir::LLVM::DISubprogramFlags subprogramFlags =
         mlir::LLVM::DISubprogramFlags{};
diff --git a/flang/test/Transforms/debug-line-table-inc-file.fir b/flang/test/Transforms/debug-line-table-inc-file.fir
index 065039b59c5ae..dcd11f75a487a 100644
--- 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_CU]], name = "sinc", linkageName = "_QPsinc", file = #[[DI_INC_FILE]], {{.*}}>
+// CHECK: #[[DI_SP:.*]] = #llvm.di_subprogram<id = distinct[{{.*}}]<>, compileUnit = #[[DI_CU]], scope = #[[DI_CU]], 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]]])