[flang][debug] Allow variable length for dummy char arguments.

[abidh]

As pointed out by @jeanPerier here, we don't need to restrict the length of the dummy character argument location to fir.unboxchar. This PR removes that restriction.

[llvmbot]

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

Author: Abid Qadeer (abidh)

Changes

As pointed out by @jeanPerier here, we don't need to restrict the length of the dummy character argument location to fir.unboxchar. This PR removes that restriction.

---

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

3 Files Affected:

• (modified) flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp (+17-16)
• (modified) flang/test/Transforms/debug-107988.fir (+1-1)
• (added) flang/test/Transforms/debug-variable-char-len.fir (+31)

diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index 1390fae062b934..4aa14ca2c2bdd6 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -295,23 +295,24 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertCharacterType(
     // 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::OpBuilder builder(context);
-        builder.setInsertionPoint(declOp);
-        mlir::Type i64Ty = builder.getIntegerType(64);
-        auto convOp = builder.create<fir::ConvertOp>(unbox.getLoc(), i64Ty,
-                                                     unbox.getResult(1));
-        mlir::LLVM::DITypeAttr Ty = convertType(i64Ty, fileAttr, scope, declOp);
-        auto lvAttr = mlir::LLVM::DILocalVariableAttr::get(
-            context, scope, name, fileAttr, /*line=*/0, /*argNo=*/0,
-            /*alignInBits=*/0, Ty, mlir::LLVM::DIFlags::Artificial);
-        builder.create<mlir::LLVM::DbgValueOp>(convOp.getLoc(), convOp, lvAttr,
-                                               nullptr);
-        varAttr = mlir::cast<mlir::LLVM::DIVariableAttr>(lvAttr);
+      auto name =
+          mlir::StringAttr::get(context, "." + declOp.getUniqName().str());
+      mlir::OpBuilder builder(context);
+      builder.setInsertionPoint(declOp);
+      mlir::Value sizeVal = declOp.getTypeparams()[0];
+      mlir::Type type = sizeVal.getType();
+      if (!mlir::isa<mlir::IntegerType>(type) || !type.isSignlessInteger()) {
+        type = builder.getIntegerType(64);
+        sizeVal =
+            builder.create<fir::ConvertOp>(declOp.getLoc(), type, sizeVal);
       }
+      mlir::LLVM::DITypeAttr Ty = convertType(type, fileAttr, scope, declOp);
+      auto lvAttr = mlir::LLVM::DILocalVariableAttr::get(
+          context, scope, name, fileAttr, /*line=*/0, /*argNo=*/0,
+          /*alignInBits=*/0, Ty, mlir::LLVM::DIFlags::Artificial);
+      builder.create<mlir::LLVM::DbgValueOp>(declOp.getLoc(), sizeVal, lvAttr,
+                                             nullptr);
+      varAttr = mlir::cast<mlir::LLVM::DIVariableAttr>(lvAttr);
     }
   }
 
diff --git a/flang/test/Transforms/debug-107988.fir b/flang/test/Transforms/debug-107988.fir
index 308f78a865120c..0b08cf1c0b2eb6 100644
--- a/flang/test/Transforms/debug-107988.fir
+++ b/flang/test/Transforms/debug-107988.fir
@@ -17,7 +17,7 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
 // CHECK: func.func @test
 // CHECK: %[[V1:.*]]:2 = fir.unboxchar{{.*}}
 // CHECK: %[[V2:.*]] = fir.convert %[[V1]]#1 : (index) -> i64
-// CHECK: llvm.intr.dbg.value #di_local_variable = %[[V2]] : i64
+// CHECK: llvm.intr.dbg.value #[[VAR]] = %[[V2]] : i64
 // CHECK: #[[STR_TY:.*]] = #llvm.di_string_type<tag = DW_TAG_string_type, name = "", stringLength = #[[VAR]], encoding = DW_ATE_ASCII>
 // CHECK: #llvm.di_local_variable<{{.*}}name = "str"{{.*}}type = #[[STR_TY]]>
 
diff --git a/flang/test/Transforms/debug-variable-char-len.fir b/flang/test/Transforms/debug-variable-char-len.fir
new file mode 100644
index 00000000000000..598d97cee970a4
--- /dev/null
+++ b/flang/test/Transforms/debug-variable-char-len.fir
@@ -0,0 +1,31 @@
+// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s -o - | FileCheck %s
+
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
+  func.func @foo(%arg0: !fir.ref<!fir.char<1,?>> {fir.bindc_name = "str1"} , %arg1: !fir.ref<i64> {fir.bindc_name = "len1"} loc("/home/haqadeer/work/fortran/t1/../str.f90":1:1), %arg2: i64) {
+    %0 = fir.emboxchar %arg0, %arg2 : (!fir.ref<!fir.char<1,?>>, i64) -> !fir.boxchar<1>
+    %c4_i32 = arith.constant 4 : i32
+    %c6_i32 = arith.constant 6 : i32
+    %c0_i64 = arith.constant 0 : i64
+    %1 = fir.undefined !fir.dscope
+    %2 = fircg.ext_declare %arg1 dummy_scope %1 {uniq_name = "_QFfooElen1"} : (!fir.ref<i64>, !fir.dscope) -> !fir.ref<i64> loc(#loc1)
+    %3:2 = fir.unboxchar %0 : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
+    %4 = fir.load %2 : !fir.ref<i64>
+    %5 = arith.cmpi sgt, %4, %c0_i64 : i64
+    %6 = arith.select %5, %4, %c0_i64 : i64
+    %7 = fircg.ext_declare %3#0 typeparams %6 dummy_scope %1 {uniq_name = "_QFfooEstr1"} : (!fir.ref<!fir.char<1,?>>, i64, !fir.dscope) -> !fir.ref<!fir.char<1,?>> loc(#loc2)
+    return
+  } loc(#loc3)
+}
+
+
+#loc1 = loc("test.f90":18:1)
+#loc2 = loc("test.f90":17:1)
+#loc3 = loc("test.f90":15:1)
+
+// CHECK: #[[VAR:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFfooEstr1"{{.*}}flags = Artificial>
+// CHECK: func.func @foo
+// CHECK: llvm.intr.dbg.value #[[VAR]]
+// CHECK: return
+// CHECK: #[[STR_TY:.*]] = #llvm.di_string_type<tag = DW_TAG_string_type, name = "", stringLength = #[[VAR]], encoding = DW_ATE_ASCII>
+// CHECK: #llvm.di_local_variable<{{.*}}name = "str1"{{.*}}type = #[[STR_TY]]>
+

[jeanPerier]

LGTM, thanks for the follow up