[flang-commits] [flang] [flang][debug] Improve handling of dummy character arguments. (PR #108283)
Abid Qadeer via flang-commits
flang-commits at lists.llvm.org
Tue Sep 17 07:10:28 PDT 2024
https://github.com/abidh updated https://github.com/llvm/llvm-project/pull/108283
>From 0a01feada5521dba3ed0a1e4b7f31f64ec6d68b6 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Tue, 3 Sep 2024 10:21:49 +0100
Subject: [PATCH 1/3] [flang][debug] Improve handling of dummy character
arguments.
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.
---
flang/lib/Optimizer/CodeGen/CodeGen.cpp | 9 ++++++
.../Transforms/DebugTypeGenerator.cpp | 23 +++++++++++++--
flang/test/Transforms/debug-107988.fir | 29 +++++++++++++++++++
3 files changed, 59 insertions(+), 2 deletions(-)
create mode 100644 flang/test/Transforms/debug-107988.fir
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 82c6a6618e0ed8..327ce5b6ea5e79 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -271,6 +271,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;
@@ -289,7 +290,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
@@ -299,7 +318,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]]
>From 3f29cef4261a2ee48eca763d29130e83f42f4396 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Tue, 17 Sep 2024 14:57:30 +0100
Subject: [PATCH 2/3] Handle review comment.
Instead of attaching variable to UnboxCharOp and then generating
DbgValueOp in the Codegen, we now generate it in AddDebugInfo pass. It
removes the need to carry the variable till the Codegen.
---
flang/lib/Optimizer/CodeGen/CodeGen.cpp | 9 ---------
.../Optimizer/Transforms/DebugTypeGenerator.cpp | 12 ++++++++----
flang/test/Transforms/debug-107988.fir | 14 ++++----------
3 files changed, 12 insertions(+), 23 deletions(-)
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index c65e7d40aa7c1e..88293bcf36a780 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -3252,15 +3252,6 @@ 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 327ce5b6ea5e79..9415d41495fc0b 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -299,13 +299,17 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertCharacterType(
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);
+ 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);
- mlir::OpBuilder builder(context);
- unbox->setLoc(builder.getFusedLoc({unbox->getLoc()}, lvAttr));
+ builder.create<mlir::LLVM::DbgValueOp>(convOp.getLoc(), convOp, 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 0b9a1e7a4be2ee..308f78a865120c 100644
--- a/flang/test/Transforms/debug-107988.fir
+++ b/flang/test/Transforms/debug-107988.fir
@@ -1,7 +1,4 @@
// 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) {
@@ -16,14 +13,11 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
#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: 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: #[[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]]
>From 09cb41714ca42bfd51eeb3faa428545a3ade82e7 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Tue, 17 Sep 2024 15:09:05 +0100
Subject: [PATCH 3/3] Update comment to address a review comment.
---
flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index 9415d41495fc0b..1390fae062b934 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -290,10 +290,10 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertCharacterType(
sizeInBits =
charTy.getLen() * kindMapping.getCharacterBitsize(charTy.getFKind());
} else {
- // 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.
+ // In assumed length string, 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)) {
More information about the flang-commits
mailing list