[flang-commits] [flang] [flang][debug] Fix array lower bounds in derived type members. (PR #113183)

Abid Qadeer via flang-commits flang-commits at lists.llvm.org
Mon Oct 21 08:49:24 PDT 2024


https://github.com/abidh created https://github.com/llvm/llvm-project/pull/113183

The lower bound information for the array members of a derived type can't be obtained from the `DeclareOp`. It has to be extracted from the `TypeInfoOp`. That was left as FIXME in the code. This PR adds the missing functionality to fix the issue.

I tried the following approaches before settling on the current one that is to generate `DITypeAttr` for array members right where the components are being processed.

1. Generate a temp XDeclareOp with the shift information obtained from the `TypeInfoOp`. This caused a few issues mostly related to `unrealized_conversion_cast`.

2. Change the shift operands in the `declOp` that was passed in the function before calling `convertType`. The code can be seen in the abcf031a8e5a02f0081e7f293858302e7bf47bec. It essentially looked like the following. It works correctly but I was not sure if temporarily changing the `declOp` is the safe thing to do.

```
mlir::OperandRange originalShift = declOp.getShift();
mlir::MutableOperandRange mutableOpRange = declOp.getShiftMutable();
mutableOpRange.assign(shiftOpers);
elemTy = convertType(fieldTy, fileAttr, scope, declOp);
mutableOpRange.assign(originalShift);
```

Fixes #113178.

>From abcf031a8e5a02f0081e7f293858302e7bf47bec Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Sun, 20 Oct 2024 13:36:38 +0100
Subject: [PATCH 1/2] Fix array lower bounds in derived type members.

---
 .../Transforms/DebugTypeGenerator.cpp         | 30 +++++++++++++++++--
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index 71e534b4f2e2a3..18a47069f51691 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -15,6 +15,7 @@
 #include "DebugTypeGenerator.h"
 #include "flang/Optimizer/CodeGen/DescriptorModel.h"
 #include "flang/Optimizer/Support/InternalNames.h"
+#include "flang/Optimizer/Support/Utils.h"
 #include "mlir/Pass/Pass.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/BinaryFormat/Dwarf.h"
@@ -298,6 +299,8 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertRecordType(
   fir::TypeInfoOp tiOp = symbolTable->lookup<fir::TypeInfoOp>(Ty.getName());
   unsigned line = (tiOp) ? getLineFromLoc(tiOp.getLoc()) : 1;
 
+  mlir::OpBuilder builder(context);
+  mlir::IntegerType intTy = mlir::IntegerType::get(context, 64);
   std::uint64_t offset = 0;
   for (auto [fieldName, fieldTy] : Ty.getTypeList()) {
     mlir::Type llvmTy;
@@ -307,11 +310,32 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertRecordType(
     else
       llvmTy = llvmTypeConverter.convertType(fieldTy);
 
-    // FIXME: Handle non defaults array bound in derived types
     uint64_t byteSize = dataLayout->getTypeSize(llvmTy);
     unsigned short byteAlign = dataLayout->getTypeABIAlignment(llvmTy);
-    mlir::LLVM::DITypeAttr elemTy =
-        convertType(fieldTy, fileAttr, scope, /*declOp=*/nullptr);
+    std::optional<llvm::ArrayRef<int64_t>> lowerBounds =
+        fir::getComponentLowerBoundsIfNonDefault(Ty, fieldName, module,
+                                                 symbolTable);
+
+    // For members of the derived types, the information about the shift in
+    // lower bounds is not part of the declOp but has to be extracted from the
+    // TypeInfoOp (using getComponentLowerBoundsIfNonDefault). We then assign it
+    // temporarily to the declOp to propagate this information where it will be
+    // needed by the type conversion logic.
+    mlir::LLVM::DITypeAttr elemTy;
+    if (declOp && lowerBounds) {
+      llvm::SmallVector<mlir::Value> shiftOpers;
+      for (int64_t bound : *lowerBounds) {
+        auto constOp = builder.create<mlir::arith::ConstantOp>(
+            module.getLoc(), builder.getIntegerAttr(intTy, bound));
+        shiftOpers.push_back(constOp);
+      }
+      mlir::OperandRange originalShift = declOp.getShift();
+      mlir::MutableOperandRange mutableOpRange = declOp.getShiftMutable();
+      mutableOpRange.assign(shiftOpers);
+      elemTy = convertType(fieldTy, fileAttr, scope, declOp);
+      mutableOpRange.assign(originalShift);
+    } else
+      elemTy = convertType(fieldTy, fileAttr, scope, /*declOp=*/nullptr);
     offset = llvm::alignTo(offset, byteAlign);
     mlir::LLVM::DIDerivedTypeAttr tyAttr = mlir::LLVM::DIDerivedTypeAttr::get(
         context, llvm::dwarf::DW_TAG_member,

>From 5805903095ffc2e9549a2f02a19f72ba469f2778 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Mon, 21 Oct 2024 15:46:29 +0100
Subject: [PATCH 2/2] Avoid changing declOp and add tests.

---
 .../Transforms/DebugTypeGenerator.cpp         | 34 +++++++++++--------
 .../test/Transforms/debug-derived-type-2.fir  | 16 +++++++++
 2 files changed, 36 insertions(+), 14 deletions(-)
 create mode 100644 flang/test/Transforms/debug-derived-type-2.fir

diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index 18a47069f51691..83f33a1e3e2a37 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -315,25 +315,31 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertRecordType(
     std::optional<llvm::ArrayRef<int64_t>> lowerBounds =
         fir::getComponentLowerBoundsIfNonDefault(Ty, fieldName, module,
                                                  symbolTable);
+    auto seqTy = mlir::dyn_cast_or_null<fir::SequenceType>(fieldTy);
 
     // For members of the derived types, the information about the shift in
     // lower bounds is not part of the declOp but has to be extracted from the
-    // TypeInfoOp (using getComponentLowerBoundsIfNonDefault). We then assign it
-    // temporarily to the declOp to propagate this information where it will be
-    // needed by the type conversion logic.
+    // TypeInfoOp (using getComponentLowerBoundsIfNonDefault).
     mlir::LLVM::DITypeAttr elemTy;
-    if (declOp && lowerBounds) {
-      llvm::SmallVector<mlir::Value> shiftOpers;
-      for (int64_t bound : *lowerBounds) {
-        auto constOp = builder.create<mlir::arith::ConstantOp>(
-            module.getLoc(), builder.getIntegerAttr(intTy, bound));
-        shiftOpers.push_back(constOp);
+    if (lowerBounds && seqTy &&
+        lowerBounds->size() == seqTy.getShape().size()) {
+      llvm::SmallVector<mlir::LLVM::DINodeAttr> elements;
+      for (auto [bound, dim] :
+           llvm::zip_equal(*lowerBounds, seqTy.getShape())) {
+        auto countAttr = mlir::IntegerAttr::get(intTy, llvm::APInt(64, dim));
+        auto lowerAttr = mlir::IntegerAttr::get(intTy, llvm::APInt(64, bound));
+        auto subrangeTy = mlir::LLVM::DISubrangeAttr::get(
+            context, countAttr, lowerAttr, /*upperBound=*/nullptr,
+            /*stride=*/nullptr);
+        elements.push_back(subrangeTy);
       }
-      mlir::OperandRange originalShift = declOp.getShift();
-      mlir::MutableOperandRange mutableOpRange = declOp.getShiftMutable();
-      mutableOpRange.assign(shiftOpers);
-      elemTy = convertType(fieldTy, fileAttr, scope, declOp);
-      mutableOpRange.assign(originalShift);
+      elemTy = mlir::LLVM::DICompositeTypeAttr::get(
+          context, llvm::dwarf::DW_TAG_array_type, /*name=*/nullptr,
+          /*file=*/nullptr, /*line=*/0, /*scope=*/nullptr,
+          convertType(seqTy.getEleTy(), fileAttr, scope, declOp),
+          mlir::LLVM::DIFlags::Zero, /*sizeInBits=*/0, /*alignInBits=*/0,
+          elements, /*dataLocation=*/nullptr, /*rank=*/nullptr,
+          /*allocated=*/nullptr, /*associated=*/nullptr);
     } else
       elemTy = convertType(fieldTy, fileAttr, scope, /*declOp=*/nullptr);
     offset = llvm::alignTo(offset, byteAlign);
diff --git a/flang/test/Transforms/debug-derived-type-2.fir b/flang/test/Transforms/debug-derived-type-2.fir
new file mode 100644
index 00000000000000..63e842619edbec
--- /dev/null
+++ b/flang/test/Transforms/debug-derived-type-2.fir
@@ -0,0 +1,16 @@
+// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s
+
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
+  fir.global @_QMmEvar : !fir.type<_QMmTt1{elm:!fir.array<5xi32>,elm2:!fir.array<5x8xi32>}> {} loc(#loc1)
+  fir.type_info @_QMmTt1 noinit nodestroy nofinal : !fir.type<_QMmTt1{elm:!fir.array<5xi32>,elm2:!fir.array<5x8xi32>}> component_info {
+    fir.dt_component "elm" lbs [2]
+    fir.dt_component "elm2" lbs [1, 3]
+  } loc(#loc1)
+}
+#loc1 = loc("derived.f90":24:1)
+
+
+// CHECK-DAG: #[[TY1:.*]] = #llvm.di_composite_type<tag = DW_TAG_array_type, {{.*}} = #{{.*}}, elements = #llvm.di_subrange<count = 5 : i64, lowerBound = 2 : i64>>
+// CHECK-DAG: #[[TY2:.*]] = #llvm.di_composite_type<tag = DW_TAG_array_type, baseType = #{{.*}}, elements = #llvm.di_subrange<count = 5 : i64, lowerBound = 1 : i64>, #llvm.di_subrange<count = 8 : i64, lowerBound = 3 : i64>>
+// CHECK-DAG: #llvm.di_derived_type<tag = DW_TAG_member, name = "elm", baseType = #[[TY1]], sizeInBits = {{.*}}, alignInBits = {{.*}}>
+// CHECK-DAG: #llvm.di_derived_type<tag = DW_TAG_member, name = "elm2", baseType = #[[TY2]], sizeInBits = {{.*}}, alignInBits = {{.*}}>



More information about the flang-commits mailing list