[flang-commits] [flang] [flang][debug] Improve handling of dummy character arguments. (PR #108283)

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-flang-codegen

Author: Abid Qadeer (abidh)

<details>
<summary>Changes</summary>

As described in #<!-- -->107998, we were not handling the case well when length of the character is not part of the type. This PR handles one of the case when the length can be calculated by looking at the result of corresponding `fir.unboxchar`.

The DIStringTypeAttr have a `stringLength` field that can be a variable. We create an artificial variable that will hold the length and used as value of `stringLength` field.

As we don't have any `fircg.ext_declare` to which we can attach the location. This PR attaches it to `fir.unboxchar` and we check for it during codegen and generate a `DbgValueOp`. I thought about the idea of generating a `fircg.ext_declare` so that we can attach the variable to it. That will avoid touching `fir.unboxchar` in codegen. But it would also have required adding an `alloca` to hold the value so that we can use `DbgDeclareOp`. I opted for `fir.unboxchar` as it seemed simpler.

Fixes #<!-- -->107998.

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


3 Files Affected:

- (modified) flang/lib/Optimizer/CodeGen/CodeGen.cpp (+9) 
- (modified) flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp (+21-2) 
- (added) flang/test/Transforms/debug-107988.fir (+29) 


``````````diff
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index 88293bcf36a780..c65e7d40aa7c1e 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -3252,6 +3252,15 @@ struct UnboxCharOpConversion : public fir::FIROpConversion<fir::UnboxCharOp> {
     auto len = rewriter.create<mlir::LLVM::ExtractValueOp>(loc, tuple, 1);
     mlir::Value lenAfterCast = integerCast(loc, rewriter, lenTy, len);
 
+    // If a variable is attached to fir.unboxchar then generate a DbgValueOp
+    if (auto fusedLoc = mlir::dyn_cast<mlir::FusedLoc>(unboxchar.getLoc())) {
+      if (auto varAttr =
+              mlir::dyn_cast_or_null<mlir::LLVM::DILocalVariableAttr>(
+                  fusedLoc.getMetadata())) {
+        rewriter.create<mlir::LLVM::DbgValueOp>(lenAfterCast.getLoc(),
+                                                lenAfterCast, varAttr, nullptr);
+      }
+    }
     rewriter.replaceOp(unboxchar,
                        llvm::ArrayRef<mlir::Value>{ptrToBuffer, lenAfterCast});
     return mlir::success();
diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index 7c4382079fd6d4..eba2aad1b0851e 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -268,6 +268,7 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertCharacterType(
   uint64_t sizeInBits = 0;
   mlir::LLVM::DIExpressionAttr lenExpr = nullptr;
   mlir::LLVM::DIExpressionAttr locExpr = nullptr;
+  mlir::LLVM::DIVariableAttr varAttr = nullptr;
 
   if (hasDescriptor) {
     llvm::SmallVector<mlir::LLVM::DIExpressionElemAttr> ops;
@@ -286,7 +287,25 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertCharacterType(
     sizeInBits =
         charTy.getLen() * kindMapping.getCharacterBitsize(charTy.getFKind());
   } else {
-    return genPlaceholderType(context);
+    // In some situations, the len of the character is not part of the type but
+    // can be found at the runtime. Here we create an artificial variable that
+    // will contain that length. This variable is used as 'stringLength' in
+    // DIStringTypeAttr.
+    if (declOp && !declOp.getTypeparams().empty()) {
+      mlir::Operation *op = declOp.getTypeparams()[0].getDefiningOp();
+      if (auto unbox = mlir::dyn_cast_or_null<fir::UnboxCharOp>(op)) {
+        auto name =
+            mlir::StringAttr::get(context, "." + declOp.getUniqName().str());
+        mlir::LLVM::DITypeAttr Ty =
+            convertType(unbox.getResult(1).getType(), fileAttr, scope, declOp);
+        auto lvAttr = mlir::LLVM::DILocalVariableAttr::get(
+            context, scope, name, fileAttr, /*line=*/0, /*argNo=*/0,
+            /*alignInBits=*/0, Ty, mlir::LLVM::DIFlags::Artificial);
+        mlir::OpBuilder builder(context);
+        unbox->setLoc(builder.getFusedLoc({unbox->getLoc()}, lvAttr));
+        varAttr = mlir::cast<mlir::LLVM::DIVariableAttr>(lvAttr);
+      }
+    }
   }
 
   // FIXME: Currently the DIStringType in llvm does not have the option to set
@@ -296,7 +315,7 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertCharacterType(
   return mlir::LLVM::DIStringTypeAttr::get(
       context, llvm::dwarf::DW_TAG_string_type,
       mlir::StringAttr::get(context, ""), sizeInBits, /*alignInBits=*/0,
-      /*stringLength=*/nullptr, lenExpr, locExpr, encoding);
+      /*stringLength=*/varAttr, lenExpr, locExpr, encoding);
 }
 
 mlir::LLVM::DITypeAttr DebugTypeGenerator::convertPointerLikeType(
diff --git a/flang/test/Transforms/debug-107988.fir b/flang/test/Transforms/debug-107988.fir
new file mode 100644
index 00000000000000..0b9a1e7a4be2ee
--- /dev/null
+++ b/flang/test/Transforms/debug-107988.fir
@@ -0,0 +1,29 @@
+// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s -o - | FileCheck %s
+// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s -o - | \
+// RUN: fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" \
+// RUN: --mlir-print-debuginfo  | FileCheck --check-prefix=CODEGEN %s
+
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
+  func.func @test(%arg0: !fir.ref<!fir.char<1,?>> {fir.bindc_name = "str"}, %arg1: i64) {
+    %0 = fir.emboxchar %arg0, %arg1 : (!fir.ref<!fir.char<1,?>>, i64) -> !fir.boxchar<1>
+    %1 = fir.undefined !fir.dscope
+    %2:2 = fir.unboxchar %0 : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)  loc(#loc1)
+    %3 = fircg.ext_declare %2#0 typeparams %2#1 dummy_scope %1 {uniq_name = "_QFtestEstr"} : (!fir.ref<!fir.char<1,?>>, index, !fir.dscope) -> !fir.ref<!fir.char<1,?>> loc(#loc1)
+    return
+  } loc(#loc2)
+}
+
+#loc1 = loc("test.f90":5:1)
+#loc2 = loc("test.f90":15:1)
+
+
+// CHECK: fir.unboxchar{{.*}} loc(#[[LOC1:.*]])
+// CHECK: #[[LOC2:.*]] = loc("test.f90":5:1)
+// CHECK: #[[VAR:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFtestEstr"{{.*}}flags = Artificial>
+// CHECK: #[[STR_TY:.*]] = #llvm.di_string_type<tag = DW_TAG_string_type, name = "", stringLength = #[[VAR]], encoding = DW_ATE_ASCII>
+// CHECK: #[[LOC1]] = loc(fused<#di_local_variable>[#[[LOC2]]])
+// CHECK: #llvm.di_local_variable<{{.*}}name = "str"{{.*}}type = #[[STR_TY]]>
+
+// CODEGEN: #[[VAR1:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFtestEstr"{{.*}}flags = Artificial>
+// CODEGEN: llvm.func @test
+// CODEGEN: llvm.intr.dbg.value #[[VAR1]]

``````````

</details>


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