From c5217900afc71ae58613ec0a5bff9db6b69e7108 Mon Sep 17 00:00:00 2001 From: Abid Qadeer Date: Mon, 30 Sep 2024 18:33:54 +0100 Subject: [PATCH 1/3] [flang][debug] Handle array types with variable size/bounds. The debug information generated by flang did not handle the cases where dimension or lower bounds of the arrays were variable. This PR fixes this issue. It will help distinguish assumed size arrays from cases where array size are variable. It also handles the variable lower bounds for assumed shape arrays. Fixes #98879. --- .../Transforms/DebugTypeGenerator.cpp | 103 +++++++++++------- .../Optimizer/Transforms/DebugTypeGenerator.h | 5 + .../Integration/debug-assumed-size-array.f90 | 2 +- .../Integration/debug-fixed-array-type-2.f90 | 4 +- .../Integration/debug-variable-array-dim.f90 | 19 ++++ flang/test/Transforms/debug-107988.fir | 2 +- .../debug-assumed-shape-array-2.fir | 23 ++++ .../Transforms/debug-assumed-size-array.fir | 2 +- .../Transforms/debug-fixed-array-type.fir | 6 +- .../Transforms/debug-variable-array-dim.fir | 41 +++++++ .../Transforms/debug-variable-char-len.fir | 2 +- 11 files changed, 160 insertions(+), 49 deletions(-) create mode 100644 flang/test/Integration/debug-variable-array-dim.f90 create mode 100644 flang/test/Transforms/debug-assumed-shape-array-2.fir create mode 100644 flang/test/Transforms/debug-variable-array-dim.fir diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp index 29e61d505bf6ad..496bd37238bd22 100644 --- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp +++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp @@ -78,6 +78,31 @@ static mlir::LLVM::DITypeAttr genPlaceholderType(mlir::MLIRContext *context) { /*bitSize=*/32, llvm::dwarf::DW_ATE_signed); } +mlir::LLVM::DILocalVariableAttr DebugTypeGenerator::generateArtificialVariable( + mlir::MLIRContext *context, mlir::Value Val, + mlir::LLVM::DIFileAttr fileAttr, mlir::LLVM::DIScopeAttr scope, + fir::cg::XDeclareOp declOp) { + // There can be multiple artificial variable for a single declOp. To help + // distinguish then, we pad the name with a counter. This static variable + // helps us achieve that. + static unsigned varID = 0; + mlir::OpBuilder builder(context); + auto name = mlir::StringAttr::get(context, "." + declOp.getUniqName().str() + + std::to_string(varID++)); + builder.setInsertionPoint(declOp); + mlir::Type type = Val.getType(); + if (!mlir::isa(type) || !type.isSignlessInteger()) { + type = builder.getIntegerType(64); + Val = builder.create(declOp.getLoc(), type, Val); + } + 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(declOp.getLoc(), Val, lvAttr, nullptr); + return lvAttr; +} + mlir::LLVM::DITypeAttr DebugTypeGenerator::convertBoxedSequenceType( fir::SequenceType seqTy, mlir::LLVM::DIFileAttr fileAttr, mlir::LLVM::DIScopeAttr scope, fir::cg::XDeclareOp declOp, @@ -122,11 +147,12 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertBoxedSequenceType( mlir::Attribute lowerAttr = nullptr; // If declaration has a lower bound, use it. if (declOp && declOp.getShift().size() > index) { - // TODO: Handle case where lower bound is a variable (instead of a - // constant as handled here) if (std::optional optint = getIntIfConstant(declOp.getShift()[index])) lowerAttr = mlir::IntegerAttr::get(intTy, llvm::APInt(64, *optint)); + else + lowerAttr = generateArtificialVariable( + context, declOp.getShift()[index], fileAttr, scope, declOp); } // FIXME: If `indexSize` happens to be bigger than address size on the // system then we may have to change 'DW_OP_deref' here. @@ -320,31 +346,43 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertSequenceType( unsigned index = 0; auto intTy = mlir::IntegerType::get(context, 64); for (fir::SequenceType::Extent dim : seqTy.getShape()) { - int64_t shift = 1; + mlir::Attribute lowerAttr = nullptr; + mlir::Attribute countAttr = nullptr; + // If declOp is present, we use the shift in it to get the lower bound of + // the array. If it is constant, that is used. If it is not constant, we + // create a variable that represents its location and use that as lower + // bound. As an optimization, we don't create a lower bound when shift is a + // constant 1 as that is the default. if (declOp && declOp.getShift().size() > index) { if (std::optional optint = - getIntIfConstant(declOp.getShift()[index])) - shift = *optint; + getIntIfConstant(declOp.getShift()[index])) { + if (*optint != 1) + lowerAttr = mlir::IntegerAttr::get(intTy, llvm::APInt(64, *optint)); + } else + lowerAttr = generateArtificialVariable( + context, declOp.getShift()[index], fileAttr, scope, declOp); } + if (dim == seqTy.getUnknownExtent()) { - mlir::IntegerAttr lowerAttr = nullptr; - if (declOp && declOp.getShift().size() > index) - lowerAttr = mlir::IntegerAttr::get(intTy, llvm::APInt(64, shift)); - // FIXME: This path is taken for assumed size arrays but also for arrays - // with non constant extent. For the latter case, the DISubrangeAttr - // should point to a variable which will have the extent at runtime. - auto subrangeTy = mlir::LLVM::DISubrangeAttr::get( - context, /*count=*/nullptr, lowerAttr, /*upperBound*/ nullptr, - /*stride*/ nullptr); - elements.push_back(subrangeTy); - } else { - auto countAttr = mlir::IntegerAttr::get(intTy, llvm::APInt(64, dim)); - auto lowerAttr = mlir::IntegerAttr::get(intTy, llvm::APInt(64, shift)); - auto subrangeTy = mlir::LLVM::DISubrangeAttr::get( - context, countAttr, lowerAttr, /*upperBound=*/nullptr, - /*stride=*/nullptr); - elements.push_back(subrangeTy); - } + // This path is taken for both assumed size array or when the size of the + // array is variable. In the case of variable size, we create a variable + // to use as countAttr. Note that fir has a constant size of -1 for + // assumed size array. So !optint check makes sure we don't generate + // variable in that case. + if (declOp && declOp.getShape().size() > index) { + std::optional optint = + getIntIfConstant(declOp.getShape()[index]); + if (!optint) + countAttr = generateArtificialVariable( + context, declOp.getShape()[index], fileAttr, scope, declOp); + } + } else + countAttr = mlir::IntegerAttr::get(intTy, llvm::APInt(64, dim)); + + auto subrangeTy = mlir::LLVM::DISubrangeAttr::get( + context, countAttr, lowerAttr, /*upperBound=*/nullptr, + /*stride=*/nullptr); + elements.push_back(subrangeTy); ++index; } // Apart from arrays, the `DICompositeTypeAttr` is used for other things like @@ -400,23 +438,8 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertCharacterType( // variable that will contain that length. This variable is used as // 'stringLength' in DIStringTypeAttr. if (declOp && !declOp.getTypeparams().empty()) { - 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(type) || !type.isSignlessInteger()) { - type = builder.getIntegerType(64); - sizeVal = - builder.create(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(declOp.getLoc(), sizeVal, lvAttr, - nullptr); + mlir::LLVM::DILocalVariableAttr lvAttr = generateArtificialVariable( + context, declOp.getTypeparams()[0], fileAttr, scope, declOp); varAttr = mlir::cast(lvAttr); } } diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h index b8a068e5ba148b..ff600e751eb0b1 100644 --- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h +++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h @@ -64,6 +64,11 @@ class DebugTypeGenerator { fir::cg::XDeclareOp declOp, bool genAllocated, bool genAssociated); + mlir::LLVM::DILocalVariableAttr + generateArtificialVariable(mlir::MLIRContext *context, mlir::Value Val, + mlir::LLVM::DIFileAttr fileAttr, + mlir::LLVM::DIScopeAttr scope, + fir::cg::XDeclareOp declOp); mlir::ModuleOp module; mlir::SymbolTable *symbolTable; diff --git a/flang/test/Integration/debug-assumed-size-array.f90 b/flang/test/Integration/debug-assumed-size-array.f90 index efa53e643ecbba..2d7edf62339b01 100644 --- a/flang/test/Integration/debug-assumed-size-array.f90 +++ b/flang/test/Integration/debug-assumed-size-array.f90 @@ -12,7 +12,7 @@ end module helper ! CHECK-DAG: ![[TY1:[0-9]+]] = !DICompositeType(tag: DW_TAG_array_type{{.*}}elements: ![[ELEMS1:[0-9]+]]{{.*}}) ! CHECK-DAG: ![[ELEMS1]] = !{![[ELM1:[0-9]+]], ![[EMPTY:[0-9]+]]} -! CHECK-DAG: ![[ELM1]] = !DISubrange(count: 5, lowerBound: 1) +! CHECK-DAG: ![[ELM1]] = !DISubrange(count: 5) ! CHECK-DAG: ![[EMPTY]] = !DISubrange() ! CHECK-DAG: ![[TY2:[0-9]+]] = !DICompositeType(tag: DW_TAG_array_type{{.*}}elements: ![[ELEMS2:[0-9]+]]{{.*}}) ! CHECK-DAG: ![[ELEMS2]] = !{![[EMPTY:[0-9]+]]} diff --git a/flang/test/Integration/debug-fixed-array-type-2.f90 b/flang/test/Integration/debug-fixed-array-type-2.f90 index 705c1da593c705..009b99af103253 100644 --- a/flang/test/Integration/debug-fixed-array-type-2.f90 +++ b/flang/test/Integration/debug-fixed-array-type-2.f90 @@ -23,11 +23,11 @@ function fn1(a1, b1, c1) result (res) ! CHECK-DAG: ![[INT:.*]] = !DIBasicType(name: "integer", size: 32, encoding: DW_ATE_signed) ! CHECK-DAG: ![[REAL:.*]] = !DIBasicType(name: "real", size: 32, encoding: DW_ATE_float) -! CHECK-DAG: ![[R1:.*]] = !DISubrange(count: 3, lowerBound: 1) +! CHECK-DAG: ![[R1:.*]] = !DISubrange(count: 3) ! CHECK-DAG: ![[SUB1:.*]] = !{![[R1]]} ! CHECK-DAG: ![[D1TY:.*]] = !DICompositeType(tag: DW_TAG_array_type, baseType: ![[INT]], elements: ![[SUB1]]) -! CHECK-DAG: ![[R21:.*]] = !DISubrange(count: 4, lowerBound: 1) +! CHECK-DAG: ![[R21:.*]] = !DISubrange(count: 4) ! CHECK-DAG: ![[R22:.*]] = !DISubrange(count: 5, lowerBound: -1) ! CHECK-DAG: ![[SUB2:.*]] = !{![[R21]], ![[R22]]} ! CHECK-DAG: ![[D2TY:.*]] = !DICompositeType(tag: DW_TAG_array_type, baseType: ![[INT]], elements: ![[SUB2]]) diff --git a/flang/test/Integration/debug-variable-array-dim.f90 b/flang/test/Integration/debug-variable-array-dim.f90 new file mode 100644 index 00000000000000..56f6b09d1ef8b5 --- /dev/null +++ b/flang/test/Integration/debug-variable-array-dim.f90 @@ -0,0 +1,19 @@ +! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck %s + +subroutine foo(a, n, m, p) + integer n, m, p + integer :: a(n:m, p) + a(1, 2) = 10 + print *, a +end subroutine foo + + +! CHECK-DAG: ![[VAR0:.*]] = !DILocalVariable(name: "._QFfooEa0"{{.*}}scope: ![[SCOPE:[0-9]+]]{{.*}}flags: DIFlagArtificial) +! CHECK-DAG: ![[VAR1:.*]] = !DILocalVariable(name: "._QFfooEa1"{{.*}}scope: ![[SCOPE]]{{.*}}flags: DIFlagArtificial) +! CHECK-DAG: ![[VAR2:.*]] = !DILocalVariable(name: "._QFfooEa2"{{.*}}scope: ![[SCOPE]]{{.*}}flags: DIFlagArtificial) +! CHECK-DAG: ![[SR1:.*]] = !DISubrange(count: ![[VAR1]], lowerBound: ![[VAR0]]) +! CHECK-DAG: ![[SR2:.*]] = !DISubrange(count: ![[VAR2]]) +! CHECK-DAG: ![[SR:.*]] = !{![[SR1]], ![[SR2]]} +! CHECK-DAG: ![[TY:.*]] = !DICompositeType(tag: DW_TAG_array_type{{.*}}elements: ![[SR]]) +! CHECK-DAG: !DILocalVariable(name: "a"{{.*}}scope: ![[SCOPE]]{{.*}}type: ![[TY]]) + diff --git a/flang/test/Transforms/debug-107988.fir b/flang/test/Transforms/debug-107988.fir index 0b08cf1c0b2eb6..0ba4296138f503 100644 --- a/flang/test/Transforms/debug-107988.fir +++ b/flang/test/Transforms/debug-107988.fir @@ -13,7 +13,7 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} { #loc1 = loc("test.f90":5:1) #loc2 = loc("test.f90":15:1) -// CHECK: #[[VAR:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFtestEstr"{{.*}}flags = Artificial> +// 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 diff --git a/flang/test/Transforms/debug-assumed-shape-array-2.fir b/flang/test/Transforms/debug-assumed-shape-array-2.fir new file mode 100644 index 00000000000000..e989fff8208153 --- /dev/null +++ b/flang/test/Transforms/debug-assumed-shape-array-2.fir @@ -0,0 +1,23 @@ +// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s + +// Test assumed shape array with variable lower bound. + +module attributes {dlti.dl_spec = #dlti.dl_spec<>} { + func.func private @_QFPfn(%arg0: !fir.box> {fir.bindc_name = "b"}, %arg1: !fir.ref {fir.bindc_name = "n"}) attributes {} { + %c23_i32 = arith.constant 23 : i32 + %c6_i32 = arith.constant 6 : i32 + %c20_i32 = arith.constant 20 : i32 + %0 = fir.undefined !fir.dscope + %1 = fircg.ext_declare %arg1 dummy_scope %0 {uniq_name = "_QFFfnEn"} : (!fir.ref, !fir.dscope) -> !fir.ref loc(#loc1) + %2 = fir.load %1 : !fir.ref + %3 = fir.convert %2 : (i32) -> index + %4 = fircg.ext_declare %arg0 origin %3 dummy_scope %0 {uniq_name = "_QFFfnEb"} : (!fir.box>, index, !fir.dscope) -> !fir.box> loc(#loc2) + return + } loc(#loc3) +} +#loc1 = loc("test1.f90":1:1) +#loc2 = loc("test1.f90":3:16) +#loc3 = loc("test1.f90":4:16) + +// CHECK: #[[VAR:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFFfnEb0"{{.*}}flags = Artificial> +// CHECK: #llvm.di_composite_type, lowerBound = #[[VAR]], stride = #llvm.di_expression<[{{.*}}]>>, dataLocation = <[DW_OP_push_object_address, DW_OP_deref]>> diff --git a/flang/test/Transforms/debug-assumed-size-array.fir b/flang/test/Transforms/debug-assumed-size-array.fir index 9adf711a0a6f7b..892502cb64a595 100644 --- a/flang/test/Transforms/debug-assumed-size-array.fir +++ b/flang/test/Transforms/debug-assumed-size-array.fir @@ -16,7 +16,7 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} { #loc1 = loc("test.f90":3:1) #loc2 = loc("test.f90":4:1) -// CHECK-DAG: #[[TY1:.*]] = #llvm.di_composite_type, #llvm.di_subrange<>> +// CHECK-DAG: #[[TY1:.*]] = #llvm.di_composite_type, #llvm.di_subrange<>> // CHECK-DAG: #[[TY2:.*]] = #llvm.di_composite_type> // CHECK-DAG: #llvm.di_local_variable<{{.*}}name = "a1"{{.*}}type = #[[TY1]]> // CHECK-DAG: #llvm.di_local_variable<{{.*}}name = "a2"{{.*}}type = #[[TY2]]> diff --git a/flang/test/Transforms/debug-fixed-array-type.fir b/flang/test/Transforms/debug-fixed-array-type.fir index 1a7d8115908a07..a15975c7cc92af 100644 --- a/flang/test/Transforms/debug-fixed-array-type.fir +++ b/flang/test/Transforms/debug-fixed-array-type.fir @@ -31,9 +31,9 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} { // CHECK-DAG: #[[INT:.*]] = #llvm.di_basic_type // CHECK-DAG: #[[REAL:.*]] = #llvm.di_basic_type -// CHECK-DAG: #[[D1TY:.*]] = #llvm.di_composite_type> -// CHECK-DAG: #[[D2TY:.*]] = #llvm.di_composite_type, #llvm.di_subrange> -// CHECK-DAG: #[[D3TY:.*]] = #llvm.di_composite_type, #llvm.di_subrange, #llvm.di_subrange> +// CHECK-DAG: #[[D1TY:.*]] = #llvm.di_composite_type> +// CHECK-DAG: #[[D2TY:.*]] = #llvm.di_composite_type, #llvm.di_subrange> +// CHECK-DAG: #[[D3TY:.*]] = #llvm.di_composite_type, #llvm.di_subrange, #llvm.di_subrange> // CHECK-DAG: #[[D4TY:.*]] = #llvm.di_composite_type, #llvm.di_subrange> // CHECK-DAG: #llvm.di_local_variable<{{.*}}name = "d1"{{.*}}type = #[[D1TY]]> // CHECK-DAG: #llvm.di_local_variable<{{.*}}name = "d2"{{.*}}type = #[[D2TY]]> diff --git a/flang/test/Transforms/debug-variable-array-dim.fir b/flang/test/Transforms/debug-variable-array-dim.fir new file mode 100644 index 00000000000000..5d96cfe6ed2388 --- /dev/null +++ b/flang/test/Transforms/debug-variable-array-dim.fir @@ -0,0 +1,41 @@ +// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s + +module attributes {dlti.dl_spec = #dlti.dl_spec<>} { + func.func @foo_(%arg0: !fir.ref> {fir.bindc_name = "a"}, %arg1: !fir.ref {fir.bindc_name = "n"}, %arg2: !fir.ref {fir.bindc_name = "m"}, %arg3: !fir.ref {fir.bindc_name = "p"}) attributes {fir.internal_name = "_QPfoo"} { + %c5_i32 = arith.constant 5 : i32 + %c6_i32 = arith.constant 6 : i32 + %c2 = arith.constant 2 : index + %c10_i32 = arith.constant 10 : i32 + %c0 = arith.constant 0 : index + %c1 = arith.constant 1 : index + %0 = fir.undefined !fir.dscope + %1 = fircg.ext_declare %arg1 dummy_scope %0 {uniq_name = "_QFfooEn"} : (!fir.ref, !fir.dscope) -> !fir.ref loc(#loc2) + %2 = fircg.ext_declare %arg2 dummy_scope %0 {uniq_name = "_QFfooEm"} : (!fir.ref, !fir.dscope) -> !fir.ref loc(#loc3) + %3 = fircg.ext_declare %arg3 dummy_scope %0 {uniq_name = "_QFfooEp"} : (!fir.ref, !fir.dscope) -> !fir.ref loc(#loc4) + %4 = fir.load %1 : !fir.ref + %5 = fir.convert %4 : (i32) -> index + %6 = fir.load %2 : !fir.ref + %7 = fir.convert %6 : (i32) -> index + %8 = arith.subi %7, %5 : index + %9 = arith.addi %8, %c1 : index + %10 = arith.cmpi sgt, %9, %c0 : index + %11 = arith.select %10, %9, %c0 : index + %12 = fir.load %3 : !fir.ref + %13 = fir.convert %12 : (i32) -> index + %14 = arith.cmpi sgt, %13, %c0 : index + %15 = arith.select %14, %13, %c0 : index + %16 = fircg.ext_declare %arg0(%11, %15) origin %5, %c1 dummy_scope %0 {uniq_name = "_QFfooEa"} : (!fir.ref>, index, index, index, index, !fir.dscope) -> !fir.ref> loc(#loc5) + return + } loc(#loc1) +} + +#loc1 = loc("test.f90":5:1) +#loc2 = loc("test.f90":6:11) +#loc3 = loc("test.f90":7:11) +#loc4 = loc("test.f90":2:8) +#loc5 = loc("test.f90":8:11) + +// CHECK-DAG: #[[VAR0:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFfooEa0"{{.*}}flags = Artificial> +// CHECK-DAG: #[[VAR1:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFfooEa1"{{.*}}flags = Artificial> +// CHECK-DAG: #[[VAR2:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFfooEa2"{{.*}}flags = Artificial> +// CHECK-DAG: #llvm.di_composite_type, #llvm.di_subrange> diff --git a/flang/test/Transforms/debug-variable-char-len.fir b/flang/test/Transforms/debug-variable-char-len.fir index 598d97cee970a4..9e177d22d5b10e 100644 --- a/flang/test/Transforms/debug-variable-char-len.fir +++ b/flang/test/Transforms/debug-variable-char-len.fir @@ -22,7 +22,7 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} { #loc2 = loc("test.f90":17:1) #loc3 = loc("test.f90":15:1) -// CHECK: #[[VAR:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFfooEstr1"{{.*}}flags = Artificial> +// CHECK: #[[VAR:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFfooEstr1{{.*}}flags = Artificial> // CHECK: func.func @foo // CHECK: llvm.intr.dbg.value #[[VAR]] // CHECK: return From 4ec286f176a31662059017c1bc5cc8a9885a7aa3 Mon Sep 17 00:00:00 2001 From: Abid Qadeer Date: Tue, 1 Oct 2024 15:49:53 +0100 Subject: [PATCH 2/3] Add comments. --- flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp index 496bd37238bd22..3bd7d1ee39af97 100644 --- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp +++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp @@ -78,6 +78,8 @@ static mlir::LLVM::DITypeAttr genPlaceholderType(mlir::MLIRContext *context) { /*bitSize=*/32, llvm::dwarf::DW_ATE_signed); } +// Helper function to create DILocalVariableAttr and DbgValueOp when information +// about the size or dimension of a variable etc lives in an mlir::Value. mlir::LLVM::DILocalVariableAttr DebugTypeGenerator::generateArtificialVariable( mlir::MLIRContext *context, mlir::Value Val, mlir::LLVM::DIFileAttr fileAttr, mlir::LLVM::DIScopeAttr scope, From 360ae997ee735742ddfd2cccd00bf188c14533d2 Mon Sep 17 00:00:00 2001 From: Abid Qadeer Date: Thu, 3 Oct 2024 12:57:12 +0100 Subject: [PATCH 3/3] Handle review comments. Use position of operand for the variable id instead of a static counter. --- .../Transforms/DebugTypeGenerator.cpp | 18 ++++++++++-------- .../Integration/debug-variable-array-dim.f90 | 2 +- .../Transforms/debug-assumed-shape-array-2.fir | 2 +- .../Transforms/debug-variable-array-dim.fir | 2 +- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp index 3bd7d1ee39af97..94b53517762db2 100644 --- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp +++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp @@ -81,27 +81,29 @@ static mlir::LLVM::DITypeAttr genPlaceholderType(mlir::MLIRContext *context) { // Helper function to create DILocalVariableAttr and DbgValueOp when information // about the size or dimension of a variable etc lives in an mlir::Value. mlir::LLVM::DILocalVariableAttr DebugTypeGenerator::generateArtificialVariable( - mlir::MLIRContext *context, mlir::Value Val, + mlir::MLIRContext *context, mlir::Value val, mlir::LLVM::DIFileAttr fileAttr, mlir::LLVM::DIScopeAttr scope, fir::cg::XDeclareOp declOp) { // There can be multiple artificial variable for a single declOp. To help - // distinguish then, we pad the name with a counter. This static variable - // helps us achieve that. - static unsigned varID = 0; + // distinguish them, we pad the name with a counter. The counter is the + // position of 'val' in the operands of declOp. + auto varID = std::distance( + declOp.getOperands().begin(), + std::find(declOp.getOperands().begin(), declOp.getOperands().end(), val)); mlir::OpBuilder builder(context); auto name = mlir::StringAttr::get(context, "." + declOp.getUniqName().str() + - std::to_string(varID++)); + std::to_string(varID)); builder.setInsertionPoint(declOp); - mlir::Type type = Val.getType(); + mlir::Type type = val.getType(); if (!mlir::isa(type) || !type.isSignlessInteger()) { type = builder.getIntegerType(64); - Val = builder.create(declOp.getLoc(), type, Val); + val = builder.create(declOp.getLoc(), type, val); } 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(declOp.getLoc(), Val, lvAttr, nullptr); + builder.create(declOp.getLoc(), val, lvAttr, nullptr); return lvAttr; } diff --git a/flang/test/Integration/debug-variable-array-dim.f90 b/flang/test/Integration/debug-variable-array-dim.f90 index 56f6b09d1ef8b5..e68cb46b5f835a 100644 --- a/flang/test/Integration/debug-variable-array-dim.f90 +++ b/flang/test/Integration/debug-variable-array-dim.f90 @@ -8,7 +8,7 @@ subroutine foo(a, n, m, p) end subroutine foo -! CHECK-DAG: ![[VAR0:.*]] = !DILocalVariable(name: "._QFfooEa0"{{.*}}scope: ![[SCOPE:[0-9]+]]{{.*}}flags: DIFlagArtificial) +! CHECK-DAG: ![[VAR0:.*]] = !DILocalVariable(name: "._QFfooEa3"{{.*}}scope: ![[SCOPE:[0-9]+]]{{.*}}flags: DIFlagArtificial) ! CHECK-DAG: ![[VAR1:.*]] = !DILocalVariable(name: "._QFfooEa1"{{.*}}scope: ![[SCOPE]]{{.*}}flags: DIFlagArtificial) ! CHECK-DAG: ![[VAR2:.*]] = !DILocalVariable(name: "._QFfooEa2"{{.*}}scope: ![[SCOPE]]{{.*}}flags: DIFlagArtificial) ! CHECK-DAG: ![[SR1:.*]] = !DISubrange(count: ![[VAR1]], lowerBound: ![[VAR0]]) diff --git a/flang/test/Transforms/debug-assumed-shape-array-2.fir b/flang/test/Transforms/debug-assumed-shape-array-2.fir index e989fff8208153..212a3453d110dc 100644 --- a/flang/test/Transforms/debug-assumed-shape-array-2.fir +++ b/flang/test/Transforms/debug-assumed-shape-array-2.fir @@ -19,5 +19,5 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} { #loc2 = loc("test1.f90":3:16) #loc3 = loc("test1.f90":4:16) -// CHECK: #[[VAR:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFFfnEb0"{{.*}}flags = Artificial> +// CHECK: #[[VAR:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFFfnEb1"{{.*}}flags = Artificial> // CHECK: #llvm.di_composite_type, lowerBound = #[[VAR]], stride = #llvm.di_expression<[{{.*}}]>>, dataLocation = <[DW_OP_push_object_address, DW_OP_deref]>> diff --git a/flang/test/Transforms/debug-variable-array-dim.fir b/flang/test/Transforms/debug-variable-array-dim.fir index 5d96cfe6ed2388..1f401041dee575 100644 --- a/flang/test/Transforms/debug-variable-array-dim.fir +++ b/flang/test/Transforms/debug-variable-array-dim.fir @@ -35,7 +35,7 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} { #loc4 = loc("test.f90":2:8) #loc5 = loc("test.f90":8:11) -// CHECK-DAG: #[[VAR0:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFfooEa0"{{.*}}flags = Artificial> +// CHECK-DAG: #[[VAR0:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFfooEa3"{{.*}}flags = Artificial> // CHECK-DAG: #[[VAR1:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFfooEa1"{{.*}}flags = Artificial> // CHECK-DAG: #[[VAR2:.*]] = #llvm.di_local_variable<{{.*}}name = "._QFfooEa2"{{.*}}flags = Artificial> // CHECK-DAG: #llvm.di_composite_type, #llvm.di_subrange>