[flang][debug] Fix array lower bounds in derived type members.

[abidh]

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 abcf031. 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.

[llvmbot]

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

Author: Abid Qadeer (abidh)

Changes

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 abcf031. 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.

---

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

2 Files Affected:

• (modified) flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp (+33-3)
• (added) flang/test/Transforms/debug-derived-type-2.fir (+16)

diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index 71e534b4f2e2a3..83f33a1e3e2a37 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,38 @@ 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);
+    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).
+    mlir::LLVM::DITypeAttr elemTy;
+    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);
+      }
+      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);
     mlir::LLVM::DIDerivedTypeAttr tyAttr = mlir::LLVM::DIDerivedTypeAttr::get(
         context, llvm::dwarf::DW_TAG_member,
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 = {{.*}}>

[tblah]

Thanks for this. I agree that this is the cleaner approach. Modifying the shift operand and changing it back is probably safe but I wouldn't like the feel of it.

[jeanPerier]

LGTM