[flang-commits] [flang] [flang][debug] Support assumed size arrays. (PR #96316)

Abid Qadeer via flang-commits flang-commits at lists.llvm.org
Mon Jun 24 06:37:06 PDT 2024


https://github.com/abidh updated https://github.com/llvm/llvm-project/pull/96316

>From 24af777f2193be8413224f1154f1aeb44574e5d3 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Wed, 19 Jun 2024 19:21:00 +0100
Subject: [PATCH 1/3] [flang][debug] Support assumed size arrays.

Here we don't know the size of last dimension and use a null subrange
for that dimension. User will have to provide the dimension when
evaluating variable of this type. The debugging looks like as follows:

subroutine fn(a1, a2)
 integer  a1(5, *), a2(*)
...
end

(gdb) p a1(1,2)
$2 = 2
(gdb) p a2(3)
$3 = 3
(gdb) ptype a1
type = integer (5,*)
(gdb) ptype a2
type = integer (*)
---
 .../Transforms/DebugTypeGenerator.cpp         | 35 ++++++++++---------
 .../Integration/debug-assumed-size-array.f90  | 20 +++++++++++
 .../Transforms/debug-assumed-size-array.fir   | 21 +++++++++++
 3 files changed, 60 insertions(+), 16 deletions(-)
 create mode 100644 flang/test/Integration/debug-assumed-size-array.f90
 create mode 100644 flang/test/Transforms/debug-assumed-size-array.fir

diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index 407ecc8e327b4..5bf938aeaee5e 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -155,28 +155,31 @@ 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;
   mlir::LLVM::DITypeAttr elemTy =
       convertType(seqTy.getEleTy(), fileAttr, scope, loc);
 
   for (fir::SequenceType::Extent 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);
+    if (dim == seqTy.getUnknownExtent()) {
+      auto subrangeTy = mlir::LLVM::DISubrangeAttr::get(
+          context, nullptr, nullptr, nullptr, nullptr);
+      elements.push_back(subrangeTy);
+    } else {
+      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
diff --git a/flang/test/Integration/debug-assumed-size-array.f90 b/flang/test/Integration/debug-assumed-size-array.f90
new file mode 100644
index 0000000000000..efa53e643ecbb
--- /dev/null
+++ b/flang/test/Integration/debug-assumed-size-array.f90
@@ -0,0 +1,20 @@
+! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck  %s
+
+module helper
+  implicit none
+  contains
+  subroutine fn (a1, a2)
+	  integer  a1(5, *), a2(*)
+    print *, a1(1,1)
+    print *, a2(2)
+  end subroutine fn
+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: ![[EMPTY]] = !DISubrange()
+! CHECK-DAG: ![[TY2:[0-9]+]] = !DICompositeType(tag: DW_TAG_array_type{{.*}}elements: ![[ELEMS2:[0-9]+]]{{.*}})
+! CHECK-DAG: ![[ELEMS2]] = !{![[EMPTY:[0-9]+]]}
+! CHECK-DAG: !DILocalVariable(name: "a1"{{.*}}type: ![[TY1:[0-9]+]])
+! CHECK-DAG: !DILocalVariable(name: "a2"{{.*}}type: ![[TY2:[0-9]+]])
diff --git a/flang/test/Transforms/debug-assumed-size-array.fir b/flang/test/Transforms/debug-assumed-size-array.fir
new file mode 100644
index 0000000000000..d25224fb1b5ec
--- /dev/null
+++ b/flang/test/Transforms/debug-assumed-size-array.fir
@@ -0,0 +1,21 @@
+// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s
+
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
+  func.func @_QMhelperPfn(%arg0: !fir.ref<!fir.array<5x?xi32>> {fir.bindc_name = "a1"}, %arg1: !fir.ref<!fir.array<?xi32>> {fir.bindc_name = "a2"}, %arg2: !fir.ref<!fir.array<2x?xi32>> {fir.bindc_name = "a3"}) {
+    %c5 = arith.constant 5 : index
+    %c1 = arith.constant 1 : index
+    %c-1 = arith.constant -1 : index
+    %0 = fir.undefined !fir.dscope
+    %1 = fircg.ext_declare %arg0(%c5, %c-1) dummy_scope %0 {uniq_name = "_QMhelperFfnEa1"} : (!fir.ref<!fir.array<5x?xi32>>, index, index, !fir.dscope) -> !fir.ref<!fir.array<5x?xi32>> loc(#loc1)
+    %2 = fircg.ext_declare %arg1(%c-1) dummy_scope %0 {uniq_name = "_QMhelperFfnEa2"} : (!fir.ref<!fir.array<?xi32>>, index, !fir.dscope) -> !fir.ref<!fir.array<?xi32>> loc(#loc2)
+    return
+  } loc(#loc3)
+}
+#loc3 = loc("test.f90":1:1)
+#loc1 = loc("test.f90":3:1)
+#loc2 = loc("test.f90":4:1)
+
+// CHECK-DAG: #[[TY1:.*]] = #llvm.di_composite_type<tag = DW_TAG_array_type{{.*}}elements = #llvm.di_subrange<count = 5 : i64, lowerBound = 1 : i64>, #llvm.di_subrange<>>
+// CHECK-DAG: #[[TY2:.*]] = #llvm.di_composite_type<tag = DW_TAG_array_type{{.*}}elements = #llvm.di_subrange<>>
+// CHECK-DAG: #llvm.di_local_variable<{{.*}}name = "a1"{{.*}}type = #[[TY1]]>
+// CHECK-DAG: #llvm.di_local_variable<{{.*}}name = "a2"{{.*}}type = #[[TY2]]>

>From 532c2b087516ba7f2ae26a426a553834acfae0cd Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Mon, 24 Jun 2024 11:47:32 +0100
Subject: [PATCH 2/3] Handle review comments.

Added comments on constant value parameters.
---
 flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index 5bf938aeaee5e..b6269bad8e51c 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -163,7 +163,8 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertSequenceType(
   for (fir::SequenceType::Extent dim : seqTy.getShape()) {
     if (dim == seqTy.getUnknownExtent()) {
       auto subrangeTy = mlir::LLVM::DISubrangeAttr::get(
-          context, nullptr, nullptr, nullptr, nullptr);
+          context, /*count=*/nullptr, /*lowerBound=*/nullptr,
+          /*upperBound*/ nullptr, /*stride*/ nullptr);
       elements.push_back(subrangeTy);
     } else {
       auto intTy = mlir::IntegerType::get(context, 64);

>From d2b928b490f1244cce272b7025158898a83cd8f5 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Mon, 24 Jun 2024 14:26:28 +0100
Subject: [PATCH 3/3] Add comments about arrays with non const extent.

Currently, the arrays with non constant extent are being handled like
assumed size arrays. Added a FIXME to mention that there is a better
way to handle them. Also added a testcase that is currently xfailed.
---
 .../Transforms/DebugTypeGenerator.cpp         |  3 +++
 .../debug-assumed-size-array-2.f90            | 22 +++++++++++++++++++
 2 files changed, 25 insertions(+)
 create mode 100644 flang/test/Integration/debug-assumed-size-array-2.f90

diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index b6269bad8e51c..0693030b459c0 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -162,6 +162,9 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertSequenceType(
 
   for (fir::SequenceType::Extent dim : seqTy.getShape()) {
     if (dim == seqTy.getUnknownExtent()) {
+      // 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, /*lowerBound=*/nullptr,
           /*upperBound*/ nullptr, /*stride*/ nullptr);
diff --git a/flang/test/Integration/debug-assumed-size-array-2.f90 b/flang/test/Integration/debug-assumed-size-array-2.f90
new file mode 100644
index 0000000000000..0deb2dbac7909
--- /dev/null
+++ b/flang/test/Integration/debug-assumed-size-array-2.f90
@@ -0,0 +1,22 @@
+! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck  %s
+
+XFAIL: *
+
+! Test that debug info for arrays with non constant extent is different from
+! assumed size arrays.
+
+module helper
+  implicit none
+  contains
+  subroutine fn (a1, n)
+    integer n
+    integer  a1(5, n)
+    print *, a1(1,1)
+  end subroutine fn
+end module helper
+
+! CHECK-DAG: ![[TY1:[0-9]+]] = !DICompositeType(tag: DW_TAG_array_type{{.*}}elements: ![[ELEMS1:[0-9]+]]{{.*}})
+! CHECK-DAG: ![[ELEMS1]] = !{![[ELM1:[0-9]+]], ![[ELM2:[0-9]+]]}
+! CHECK-DAG: ![[ELM1]] = !DISubrange(count: 5, lowerBound: 1)
+! CHECK-DAG: ![[ELM2]] = !DISubrange(count: [[VAR:[0-9]+]], lowerBound: 1)
+! CHECK-DAG: ![[VAR]] = !DILocalVariable



More information about the flang-commits mailing list