[flang][debug] Add support for fixed size arrays. #92568

abidh · 2024-05-17T16:08:35Z

This PR adds the type conversion support for fixed size arrays. Mostly mechanical changes converting dimension values to subrange fields. A limitation is that lower bound is always one for the moment as that information is missing in SequenceType.

With this change in place, I can evaluate fixed size arrays in debugger.


(gdb) p x
$1 = ((2, 3, 4, 5) (3, 4, 5, 6) (4, 5, 6, 7) (5, 6, 7, 8) (6, 7, 8, 9))
(gdb) ptype x
type = integer (4,5)

llvmbot · 2024-05-17T16:09:06Z

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

Author: Abid Qadeer (abidh)

Changes

This PR adds the type conversion support for fixed size arrays. Mostly mechanical changes converting dimension values to subrange fields. A limitation is that lower bound is always one for the moment as that information is missing in SequenceType.

With this change in place, I can evaluate fixed size arrays in debugger.


(gdb) p x
$1 = ((2, 3, 4, 5) (3, 4, 5, 6) (4, 5, 6, 7) (5, 6, 7, 8) (6, 7, 8, 9))
(gdb) ptype x
type = integer (4,5)

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

4 Files Affected:

(modified) flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp (+40)
(modified) flang/lib/Optimizer/Transforms/DebugTypeGenerator.h (+4)
(added) flang/test/Transforms/debug-fixed-array-type-2.f90 (+43)
(added) flang/test/Transforms/debug-fixed-array-type.f90 (+33)

diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index 64c6547e06e0f..be3687e5dd1a9 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -37,6 +37,44 @@ static mlir::LLVM::DITypeAttr genPlaceholderType(mlir::MLIRContext *context) {
                       llvm::dwarf::DW_ATE_signed);
 }
 
+mlir::LLVM::DITypeAttr DebugTypeGenerator::convertSequenceType(
+    fir::SequenceType seqTy, mlir::LLVM::DIFileAttr fileAttr,
+    mlir::LLVM::DIScopeAttr scope, mlir::Location loc) {
+
+  mlir::MLIRContext *context = module.getContext();
+  // FIXME: Only fixed sizes arrays handled at the moment.
+  if (seqTy.hasDynamicExtents())
+    return genPlaceholderType(context);
+
+  llvm::SmallVector<mlir::LLVM::DINodeAttr> elements;
+  auto elemTy = convertType(seqTy.getEleTy(), fileAttr, scope, loc);
+
+  for (auto dim : seqTy.getShape()) {
+    auto intTy = mlir::IntegerType::get(context, 64);
+    // FIXME: Only supporting lower bound of 1 at the moment. The
+    // 'SequenceType' has information about the shape but not the shift. In
+    // cases, where the conversion originated during the processing of
+    // 'DeclareOp', it may be possible to pass on this information. But the
+    // type conversion should ideally be based on what information present in
+    // the type class so that it works from everywhere (e.g. when it is part
+    // of a module or a derived type.)
+    auto countAttr = mlir::IntegerAttr::get(intTy, llvm::APInt(64, dim));
+    auto lowerAttr = mlir::IntegerAttr::get(intTy, llvm::APInt(64, 1));
+    auto subrangeTy = mlir::LLVM::DISubrangeAttr::get(
+        context, countAttr, lowerAttr, nullptr, nullptr);
+    elements.push_back(subrangeTy);
+  }
+  // Apart from arrays, the `DICompositeTypeAttr` is used for other things like
+  // structure types. Many of its fields which are not applicable to arrays
+  // have been set to some valid default values.
+
+  return mlir::LLVM::DICompositeTypeAttr::get(
+      context, llvm::dwarf::DW_TAG_array_type, /*recursive id*/ {},
+      /* name */ nullptr, /* file */ nullptr, /* line */ 0, /* scope */ nullptr,
+      elemTy, mlir::LLVM::DIFlags::Zero, /* sizeInBits */ 0,
+      /*alignInBits*/ 0, elements);
+}
+
 mlir::LLVM::DITypeAttr
 DebugTypeGenerator::convertType(mlir::Type Ty, mlir::LLVM::DIFileAttr fileAttr,
                                 mlir::LLVM::DIScopeAttr scope,
@@ -57,6 +95,8 @@ DebugTypeGenerator::convertType(mlir::Type Ty, mlir::LLVM::DIFileAttr fileAttr,
                         mlir::StringAttr::get(context, logTy.getMnemonic()),
                         kindMapping.getLogicalBitsize(logTy.getFKind()),
                         llvm::dwarf::DW_ATE_boolean);
+  } else if (auto seqTy = mlir::dyn_cast_or_null<fir::SequenceType>(Ty)) {
+    return convertSequenceType(seqTy, fileAttr, scope, loc);
   } else {
     // FIXME: These types are currently unhandled. We are generating a
     // placeholder type to allow us to test supported bits.
diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h
index 5a2bb201db47a..963c919d66825 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h
@@ -31,6 +31,10 @@ class DebugTypeGenerator {
                                      mlir::Location loc);
 
 private:
+  mlir::LLVM::DITypeAttr convertSequenceType(fir::SequenceType seqTy,
+                                             mlir::LLVM::DIFileAttr fileAttr,
+                                             mlir::LLVM::DIScopeAttr scope,
+                                             mlir::Location loc);
   mlir::ModuleOp module;
   KindMapping kindMapping;
 };
diff --git a/flang/test/Transforms/debug-fixed-array-type-2.f90 b/flang/test/Transforms/debug-fixed-array-type-2.f90
new file mode 100644
index 0000000000000..315525442a5bc
--- /dev/null
+++ b/flang/test/Transforms/debug-fixed-array-type-2.f90
@@ -0,0 +1,43 @@
+! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck %s
+
+program mn
+
+  integer d1(3)
+  integer d2(2, 5)
+  real d3(6, 8, 7)
+
+  i8 = fn1(d1, d2, d3)
+contains
+  function fn1(a1, b1, c1) result (res)
+    integer a1(3)
+    integer b1(2, 5)
+    real c1(6, 8, 7)
+    integer res
+    res = a1(1) + b1(1,2) + c1(3, 3, 4)
+  end function
+
+end program
+
+! 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: ![[SUB1:.*]] = !{![[R1]]}
+! CHECK-DAG: ![[D1TY:.*]] = !DICompositeType(tag: DW_TAG_array_type, baseType: ![[INT]], elements: ![[SUB1]])
+! CHECK-DAG: !DILocalVariable(name: "d1"{{.*}}type: ![[D1TY]])
+
+! CHECK-DAG: ![[R21:.*]] = !DISubrange(count: 2, lowerBound: 1)
+! 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]])
+! CHECK-DAG: !DILocalVariable(name: "d2"{{.*}}type: ![[D2TY]])
+
+! CHECK-DAG: ![[R31:.*]] = !DISubrange(count: 6, lowerBound: 1)
+! CHECK-DAG: ![[R32:.*]] = !DISubrange(count: 8, lowerBound: 1)
+! CHECK-DAG: ![[R33:.*]] = !DISubrange(count: 7, lowerBound: 1)
+! CHECK-DAG: ![[SUB3:.*]] = !{![[R31]], ![[R32]], ![[R33]]}
+! CHECK-DAG: ![[D3TY:.*]] = !DICompositeType(tag: DW_TAG_array_type, baseType: ![[REAL]], elements: ![[SUB3]])
+! CHECK-DAG: !DILocalVariable(name: "d3"{{.*}}type: ![[D3TY]])
+
+! CHECK-DAG: !DILocalVariable(name: "a1", arg: 1{{.*}}type: ![[D1TY]])
+! CHECK-DAG: !DILocalVariable(name: "b1", arg: 2{{.*}}type: ![[D2TY]])
+! CHECK-DAG: !DILocalVariable(name: "c1", arg: 3{{.*}}type: ![[D3TY]])
diff --git a/flang/test/Transforms/debug-fixed-array-type.f90 b/flang/test/Transforms/debug-fixed-array-type.f90
new file mode 100644
index 0000000000000..d62fb8729fbac
--- /dev/null
+++ b/flang/test/Transforms/debug-fixed-array-type.f90
@@ -0,0 +1,33 @@
+! RUN: %flang_fc1 -emit-fir -debug-info-kind=standalone -mmlir --mlir-print-debuginfo %s -o - | \
+! RUN: fir-opt --cg-rewrite="preserve-declare=true" --mlir-print-debuginfo | fir-opt --add-debug-info --mlir-print-debuginfo | FileCheck %s
+
+
+program mn
+  integer d1(3)
+  integer d2(2, 5)
+  real d3(6, 8, 7)
+
+  i8 = fn1(d1, d2, d3)
+contains
+  function fn1(a1, b1, c1) result (res)
+    integer a1(3)
+    integer b1(2, 5)
+
+    real c1(6, 8, 7)
+    integer res
+    res = a1(1) + b1(1,2) + c1(3, 3, 4)
+  end function
+
+end program
+
+! CHECK-DAG: #[[INT:.*]] = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "integer", sizeInBits = 32, encoding = DW_ATE_signed>
+! CHECK-DAG: #[[REAL:.*]] = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "real", sizeInBits = 32, encoding = DW_ATE_float>
+! CHECK-DAG: #[[D1TY:.*]] = #llvm.di_composite_type<tag = DW_TAG_array_type{{.*}}baseType = #[[INT]], elements = #llvm.di_subrange<count = 3 : i64, lowerBound = 1 : i64>>
+! CHECK-DAG: #[[D2TY:.*]] = #llvm.di_composite_type<tag = DW_TAG_array_type{{.*}}baseType = #[[INT]], elements = #llvm.di_subrange<count = 2 : i64, lowerBound = 1 : i64>, #llvm.di_subrange<count = 5 : i64, lowerBound = 1 : i64>>
+! CHECK-DAG: #[[D3TY:.*]] = #llvm.di_composite_type<tag = DW_TAG_array_type{{.*}}baseType = #[[REAL]], elements = #llvm.di_subrange<count = 6 : i64, lowerBound = 1 : i64>, #llvm.di_subrange<count = 8 : i64, lowerBound = 1 : i64>, #llvm.di_subrange<count = 7 : i64, lowerBound = 1 : i64>>
+! CHECK-DAG: #llvm.di_local_variable<{{.*}}name = "d1"{{.*}}type = #[[D1TY]]>
+! CHECK-DAG: #llvm.di_local_variable<{{.*}}name = "d2"{{.*}}type = #[[D2TY]]>
+! CHECK-DAG: #llvm.di_local_variable<{{.*}}name = "d3"{{.*}}type = #[[D3TY]]>
+! CHECK-DAG: #llvm.di_local_variable<{{.*}}name = "a1"{{.*}}type = #[[D1TY]]>
+! CHECK-DAG: #llvm.di_local_variable<{{.*}}name = "b1"{{.*}}type = #[[D2TY]]>
+! CHECK-DAG: #llvm.di_local_variable<{{.*}}name = "c1"{{.*}}type = #[[D3TY]]>

kiranchandramohan · 2024-05-18T12:38:16Z

flang/test/Transforms/debug-fixed-array-type-2.f90

@@ -0,0 +1,43 @@
+! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck %s


LLVM IR tests should be moved to the Integration directory. Ideally tests should test the transformations occurring in the pass.

I have moved the test to Integration folder. I add one test that checks the conversion in the AddDebugInfo and one integration test to check that our changes results in the correct debug metadata in LLVM IR format.

kiranchandramohan · 2024-05-18T12:39:35Z

flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp

+  auto elemTy = convertType(seqTy.getEleTy(), fileAttr, scope, loc);
+
+  for (auto dim : seqTy.getShape()) {


Nit: Could you name the type for these two variables?

kiranchandramohan · 2024-05-18T14:38:25Z

flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp

+    // FIXME: Only supporting lower bound of 1 at the moment. The
+    // 'SequenceType' has information about the shape but not the shift. In
+    // cases, where the conversion originated during the processing of
+    // 'DeclareOp', it may be possible to pass on this information. But the
+    // type conversion should ideally be based on what information present in
+    // the type class so that it works from everywhere (e.g. when it is part
+    // of a module or a derived type.)


Wouldn't there be a fir.declare with a shapeshift for module variables as well?

I can see an fir.declare where it is used inside FuncOp but not at the point of declaration in GlobalOp. Also if we use the info in fir.declare, we will have to pass it through all the type generation functions. It will be much cleaner if it could be added to SequenceType instead.

flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp

kiranchandramohan

LGTM.

This PR adds the type conversion support for fixes size arrays. Mostly mechanical changes converting dimension values to subrange fields.

Co-authored-by: Tom Eccles <[email protected]>

Spell the types instead of using 'auto'.

Add test that checks only the add-debug-info pass and remove the fortran test.

The llvm#92568 introduced call of DICompositeTypeAttr::get in flang. This commit adjusts the call to fix the build. I initially opened the llvm#93516 for it but was advised to add this change in this PR.

…97598a1e4 Local branch amd-gfx 17b9759 Merged main:430729d71e4e582aa04b5d30796de14404fed56a into amd-gfx:cccd99d0cdce Remote branch main 5434b04 [flang][debug] Add support for fixed size arrays. (llvm#92568)

abidh requested review from kiranchandramohan, jeanPerier, dpalermo, kiranktp and vzakhari May 17, 2024 16:08

llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels May 17, 2024

kiranchandramohan reviewed May 18, 2024

View reviewed changes

tblah reviewed May 20, 2024

View reviewed changes

flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp Outdated Show resolved Hide resolved

kiranchandramohan approved these changes May 22, 2024

View reviewed changes

abidh and others added 5 commits May 24, 2024 11:48

[flang][Debug] Add support for fixes size arrays.

a4ab879

This PR adds the type conversion support for fixes size arrays. Mostly mechanical changes converting dimension values to subrange fields.

Move IR tests to integration directory.

9e2a1c6

Update flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp

92baea5

Co-authored-by: Tom Eccles <[email protected]>

Handle review comments.

b074ddd

Spell the types instead of using 'auto'.

Handle review comments.

4d6f285

Add test that checks only the add-debug-info pass and remove the fortran test.

abidh force-pushed the array_type branch from ed6c472 to 4d6f285 Compare May 24, 2024 11:17

abidh merged commit 5434b04 into llvm:main May 24, 2024
7 checks passed

abidh mentioned this pull request May 28, 2024

[mlir] Add missing fields in DICompositeTypeAttr. #93226

Merged

abidh deleted the array_type branch June 4, 2024 09:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flang][debug] Add support for fixed size arrays. #92568

[flang][debug] Add support for fixed size arrays. #92568

Uh oh!

abidh commented May 17, 2024

Uh oh!

llvmbot commented May 17, 2024

Uh oh!

kiranchandramohan May 18, 2024

Uh oh!

abidh May 20, 2024

Uh oh!

kiranchandramohan May 18, 2024

Uh oh!

abidh May 20, 2024

Uh oh!

kiranchandramohan May 18, 2024

Uh oh!

abidh May 20, 2024

Uh oh!

Uh oh!

kiranchandramohan left a comment

Uh oh!

Uh oh!

Uh oh!

		@@ -0,0 +1,43 @@
		! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone %s -o - \| FileCheck %s

		auto elemTy = convertType(seqTy.getEleTy(), fileAttr, scope, loc);

		for (auto dim : seqTy.getShape()) {

[flang][debug] Add support for fixed size arrays. #92568

[flang][debug] Add support for fixed size arrays. #92568

Uh oh!

Conversation

abidh commented May 17, 2024

Uh oh!

llvmbot commented May 17, 2024

Uh oh!

kiranchandramohan May 18, 2024

Choose a reason for hiding this comment

Uh oh!

abidh May 20, 2024

Choose a reason for hiding this comment

Uh oh!

kiranchandramohan May 18, 2024

Choose a reason for hiding this comment

Uh oh!

abidh May 20, 2024

Choose a reason for hiding this comment

Uh oh!

kiranchandramohan May 18, 2024

Choose a reason for hiding this comment

Uh oh!

abidh May 20, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kiranchandramohan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!