[flang][debug] handle inlined dummy_scope after #167489

[jeanPerier]

#167489 did not work properly when MLIR inlining is enabled (experimental in flang, enabled with -mllvm -inline-all).

The reason is that inlining will cause several fir.dummy_scope to coexist inside a same func.func (fir.dummy_scope of inlined func.func are preserved in order to preserve the relationship between arguments of the inlined call for better aliasing deductions).

After #167489, the debug info pass creates argument debug info for all fir.declare with a fir.dummy_scope. This causes arguments from inlined calls to appear as argument of the procedure where the call was inlined.

To avoid this, only consider that fir.declare are arguments of the current function if their fir.dummy_scope is the first one created in the function (fir.dummy_scope cannot be reorder because they have write effects to the debug memory ressource, and the fir.dummy_scope of the current functions is always emitted before any calls are lowered, so before any fir.dummy_scope are inlined).

Here is an example that was broken with flang -mllvm -inline-all -g

subroutine bar(i)
  integer :: i
  call buzz(i)
end subroutine
subroutine foo(i)
  integer :: i
  call bar(i)
  call bar(i)
end subroutine

flang -g -mllvm -inline-all -c repro.f90
conflicting debug info for argument
#dbg_declare(ptr %0, !15, !DIExpression(), !14)
!13 = !DILocalVariable(name: "i", arg: 1, scope: !12, file: !1, line: 6, type: !7)
!15 = !DILocalVariable(name: "i", arg: 1, scope: !12, file: !1, line: 1, type: !7)
error: failed to create the LLVM module

[llvmbot]

@llvm/pr-subscribers-flang-fir-hlfir

Author: None (jeanPerier)

Changes

#167489 did not work properly when MLIR inlining is enabled (experimental in flang, enabled with -mllvm -inline-all).

The reason is that inlining will cause several fir.dummy_scope to coexist inside a same func.func (fir.dummy_scope of inlined func.func are preserved in order to preserve the relationship between arguments of the inlined call for better aliasing deductions).

After #167489, the debug info pass creates argument debug info for all fir.declare with a fir.dummy_scope. This causes arguments from inlined calls to appear as argument of the procedure where the call was inlined.

To avoid this, only consider that fir.declare are arguments of the current function if their fir.dummy_scope is the first one created in the function (fir.dummy_scope cannot be reorder because they have write effects to the debug memory ressource, and the fir.dummy_scope of the current functions is always emitted before any calls are lowered, so before any fir.dummy_scope are inlined).

Here is an example that was broken with flang -mllvm -inline-all -g

subroutine bar(i)
  integer :: i
  call buzz(i)
end subroutine
subroutine foo(i)
  integer :: i
  call bar(i)
  call bar(i)
end subroutine

flang -g -mllvm -inline-all -c repro.f90
conflicting debug info for argument
#dbg_declare(ptr %0, !15, !DIExpression(), !14)
!13 = !DILocalVariable(name: "i", arg: 1, scope: !12, file: !1, line: 6, type: !7)
!15 = !DILocalVariable(name: "i", arg: 1, scope: !12, file: !1, line: 1, type: !7)
error: failed to create the LLVM module

---

Full diff: https://github.com/llvm/llvm-project/pull/168039.diff

2 Files Affected:

• (modified) flang/lib/Optimizer/Transforms/AddDebugInfo.cpp (+20-4)
• (added) flang/test/Transforms/debug-dummy-argument-inline.fir (+36)

diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index 00633b4104b6c..b21cb85e1256b 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -53,7 +53,7 @@ class AddDebugInfoPass : public fir::impl::AddDebugInfoBase<AddDebugInfoPass> {
                        mlir::LLVM::DIFileAttr fileAttr,
                        mlir::LLVM::DIScopeAttr scopeAttr,
                        fir::DebugTypeGenerator &typeGen,
-                       mlir::SymbolTable *symbolTable);
+                       mlir::SymbolTable *symbolTable, mlir::Value dummyScope);
 
 public:
   AddDebugInfoPass(fir::AddDebugInfoOptions options) : Base(options) {}
@@ -206,7 +206,8 @@ void AddDebugInfoPass::handleDeclareOp(fir::cg::XDeclareOp declOp,
                                        mlir::LLVM::DIFileAttr fileAttr,
                                        mlir::LLVM::DIScopeAttr scopeAttr,
                                        fir::DebugTypeGenerator &typeGen,
-                                       mlir::SymbolTable *symbolTable) {
+                                       mlir::SymbolTable *symbolTable,
+                                       mlir::Value dummyScope) {
   mlir::MLIRContext *context = &getContext();
   mlir::OpBuilder builder(context);
   auto result = fir::NameUniquer::deconstruct(declOp.getUniqName());
@@ -230,7 +231,7 @@ void AddDebugInfoPass::handleDeclareOp(fir::cg::XDeclareOp declOp,
 
   // Get the dummy argument position from the explicit attribute.
   unsigned argNo = 0;
-  if (declOp.getDummyScope()) {
+  if (dummyScope && declOp.getDummyScope() == dummyScope) {
     if (auto argNoOpt = declOp.getDummyArgNo())
       argNo = *argNoOpt;
   }
@@ -610,6 +611,21 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
   funcOp->setLoc(builder.getFusedLoc({l}, spAttr));
   addTargetOpDISP(/*lineTableOnly=*/false, entities);
 
+  // Find the first dummy_scope definition. This is the one of the current
+  // function. The other ones may come from inlined calls. The variables inside
+  // those inlined calls should not be identified as arguments of the current
+  // function.
+  mlir::Value dummyScope;
+  funcOp.walk([&](fir::UndefOp undef) -> mlir::WalkResult {
+    // TODO: delay fir.dummy_scope translation to undefined until
+    // codegeneration. This is nicer and safer to match.
+    if (llvm::isa<fir::DummyScopeType>(undef.getType())) {
+      dummyScope = undef;
+      return mlir::WalkResult::interrupt();
+    }
+    return mlir::WalkResult::advance();
+  });
+
   funcOp.walk([&](fir::cg::XDeclareOp declOp) {
     mlir::LLVM::DISubprogramAttr spTy = spAttr;
     if (auto tOp = declOp->getParentOfType<mlir::omp::TargetOp>()) {
@@ -619,7 +635,7 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
           spTy = sp;
       }
     }
-    handleDeclareOp(declOp, fileAttr, spTy, typeGen, symbolTable);
+    handleDeclareOp(declOp, fileAttr, spTy, typeGen, symbolTable, dummyScope);
   });
   // commonBlockMap ensures that we don't create multiple DICommonBlockAttr of
   // the same name in one function. But it is ok (rather required) to create
diff --git a/flang/test/Transforms/debug-dummy-argument-inline.fir b/flang/test/Transforms/debug-dummy-argument-inline.fir
new file mode 100644
index 0000000000000..6b3a0ee4a7c18
--- /dev/null
+++ b/flang/test/Transforms/debug-dummy-argument-inline.fir
@@ -0,0 +1,36 @@
+// Tests that inlined calls arguments are not mistaken for the arguments of the
+// procedure where the calls were inlined.
+// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s -o - | FileCheck %s
+
+!CHECK: #di_local_variable = #llvm.di_local_variable<scope = #di_subprogram, name = "i", file = #di_file, line = 6, arg = 1, type = #di_basic_type>
+!CHECK: #di_local_variable1 = #llvm.di_local_variable<scope = #di_subprogram, name = "i", file = #di_file, line = 1, type = #di_basic_type>
+
+func.func @foo_(%arg0: !fir.ref<i32> {fir.bindc_name = "i"} loc("debug-dummy-argument-inline.f90":5:1)) attributes {fir.internal_name = "_QPfoo"} {
+  %0 = fir.undefined !fir.dscope loc(#loc5)
+  %1 = fircg.ext_declare %arg0 dummy_scope %0 arg 1 {uniq_name = "_QFfooEi"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32> loc(#loc6)
+  %2 = fir.undefined !fir.dscope loc(#loc11)
+  %3 = fircg.ext_declare %1 dummy_scope %2 arg 1 {uniq_name = "_QFbarEi"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32> loc(#loc12)
+  fir.call @buzz_(%3) fastmath<contract> : (!fir.ref<i32>) -> () loc(#loc13)
+  %4 = fir.undefined !fir.dscope loc(#loc14)
+  %5 = fircg.ext_declare %1 dummy_scope %4 arg 1 {uniq_name = "_QFbarEi"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32> loc(#loc15)
+  fir.call @buzz_(%5) fastmath<contract> : (!fir.ref<i32>) -> () loc(#loc16)
+  return loc(#loc9)
+} loc(#loc5)
+func.func private @buzz_(!fir.ref<i32>) attributes {fir.internal_name = "_QPbuzz"} loc(#loc10)
+#loc = loc("debug-dummy-argument-inline.f90":0:0)
+#loc1 = loc("debug-dummy-argument-inline.f90":1:1)
+#loc2 = loc("debug-dummy-argument-inline.f90":2:14)
+#loc3 = loc("debug-dummy-argument-inline.f90":3:3)
+#loc4 = loc("debug-dummy-argument-inline.f90":4:1)
+#loc5 = loc("debug-dummy-argument-inline.f90":5:1)
+#loc6 = loc("debug-dummy-argument-inline.f90":6:14)
+#loc7 = loc("debug-dummy-argument-inline.f90":7:3)
+#loc8 = loc("debug-dummy-argument-inline.f90":8:3)
+#loc9 = loc("debug-dummy-argument-inline.f90":9:1)
+#loc10 = loc("debug-dummy-argument-inline.f90":3:8)
+#loc11 = loc(callsite(#loc1 at #loc7))
+#loc12 = loc(callsite(#loc2 at #loc7))
+#loc13 = loc(callsite(#loc3 at #loc7))
+#loc14 = loc(callsite(#loc1 at #loc8))
+#loc15 = loc(callsite(#loc2 at #loc8))
+#loc16 = loc(callsite(#loc3 at #loc8))

[tblah]

Makes sense. LGTM