[flang][debug] Support assumed shape arrays. #94644
Merged
+182
−23
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
abidh
requested review from
tblah,
kiranchandramohan,
jeanPerier,
dpalermo,
jinisusan,
kiranktp and
vzakhari
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-codegen Author: Abid Qadeer (abidh) ChangesThis PR generates dwarf to extract the information about the arrays from descriptor. The DWARF needs the offset of the fields like As we use data layout now, some tests needed to be adjusted to have a dummy data layout to avoid failure. With this change in place, GDB is able show the assumed shape arrays correctly. subroutine ff(n, m, arr) Breakpoint 1, ff (n=4, m=3, arr=...) at test1.f90:13 Full diff: https://github.com/llvm/llvm-project/pull/94644.diff 14 Files Affected:
diff --git a/flang/lib/Optimizer/CodeGen/DescriptorModel.h b/flang/include/flang/Optimizer/CodeGen/DescriptorModel.h
similarity index 100%
rename from flang/lib/Optimizer/CodeGen/DescriptorModel.h
rename to flang/include/flang/Optimizer/CodeGen/DescriptorModel.h
diff --git a/flang/lib/Optimizer/CodeGen/TypeConverter.cpp b/flang/lib/Optimizer/CodeGen/TypeConverter.cpp
index 07d3bd713ce45..501a36f5b68ba 100644
--- a/flang/lib/Optimizer/CodeGen/TypeConverter.cpp
+++ b/flang/lib/Optimizer/CodeGen/TypeConverter.cpp
@@ -13,9 +13,9 @@
#define DEBUG_TYPE "flang-type-conversion"
#include "flang/Optimizer/CodeGen/TypeConverter.h"
-#include "DescriptorModel.h"
#include "flang/Common/Fortran.h"
#include "flang/Optimizer/Builder/Todo.h" // remove when TODO's are done
+#include "flang/Optimizer/CodeGen/DescriptorModel.h"
#include "flang/Optimizer/CodeGen/TBAABuilder.h"
#include "flang/Optimizer/CodeGen/Target.h"
#include "flang/Optimizer/Dialect/FIRType.h"
diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index a174f2c2bc4bf..858f58b8e2703 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -13,6 +13,10 @@
#define DEBUG_TYPE "flang-debug-type-generator"
#include "DebugTypeGenerator.h"
+#include "flang/Optimizer/CodeGen/DescriptorModel.h"
+#include "flang/Optimizer/CodeGen/TypeConverter.h"
+#include "flang/Optimizer/Support/DataLayout.h"
+#include "mlir/Pass/Pass.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/BinaryFormat/Dwarf.h"
#include "llvm/Support/Debug.h"
@@ -22,6 +26,60 @@ namespace fir {
DebugTypeGenerator::DebugTypeGenerator(mlir::ModuleOp m)
: module(m), kindMapping(getKindMapping(m)) {
LLVM_DEBUG(llvm::dbgs() << "DITypeAttr generator\n");
+
+ std::optional<mlir::DataLayout> dl =
+ fir::support::getOrSetDataLayout(module, /*allowDefaultLayout=*/true);
+ if (!dl)
+ mlir::emitError(module.getLoc(), "Missing data layout attribute in module");
+
+ mlir::MLIRContext *context = module.getContext();
+
+ // The debug information requires the offset of certain fields in the
+ // descriptors like lower_bound and extent for each dimension. The code
+ // below uses getDescFieldTypeModel to get the type representing each field
+ // and then use data layout to get its size. It adds the size to get the
+ // offset.
+ // As has been mentioned in DescriptorModel.h that code may be confusing
+ // host for the target in calculating the type of the descriptor fields. But
+ // debug info is using similar logic to what codegen is doing so it will
+ // atleast be representing the generated code correctly.
+ // My testing for a 32-bit shows that base_addr* is correctly given as a
+ // 32-bit entity. The index types are 64-bit so I am a bit uncertain how
+ // the alignment will effect the calculation of offsets in that case.
+
+ // base_addr*
+ dimsOffset =
+ dl->getTypeSizeInBits(getDescFieldTypeModel<kAddrPosInBox>()(context));
+
+ // elem_len
+ dimsOffset +=
+ dl->getTypeSizeInBits(getDescFieldTypeModel<kElemLenPosInBox>()(context));
+
+ // version
+ dimsOffset +=
+ dl->getTypeSizeInBits(getDescFieldTypeModel<kVersionPosInBox>()(context));
+
+ // rank
+ dimsOffset +=
+ dl->getTypeSizeInBits(getDescFieldTypeModel<kRankPosInBox>()(context));
+
+ // type
+ dimsOffset +=
+ dl->getTypeSizeInBits(getDescFieldTypeModel<kTypePosInBox>()(context));
+
+ // attribute
+ dimsOffset += dl->getTypeSizeInBits(
+ getDescFieldTypeModel<kAttributePosInBox>()(context));
+
+ // f18Addendum
+ dimsOffset += dl->getTypeSizeInBits(
+ getDescFieldTypeModel<kF18AddendumPosInBox>()(context));
+
+ // dims
+ dimsSize =
+ dl->getTypeSizeInBits(getDescFieldTypeModel<kDimsPosInBox>()(context));
+ dimsOffset /= 8;
+ dimsSize /= 8;
}
static mlir::LLVM::DITypeAttr genBasicType(mlir::MLIRContext *context,
@@ -37,10 +95,82 @@ static mlir::LLVM::DITypeAttr genPlaceholderType(mlir::MLIRContext *context) {
llvm::dwarf::DW_ATE_signed);
}
+mlir::LLVM::DITypeAttr DebugTypeGenerator::convertBoxedSequenceType(
+ fir::SequenceType seqTy, mlir::LLVM::DIFileAttr fileAttr,
+ mlir::LLVM::DIScopeAttr scope, mlir::Location loc, bool genAllocated,
+ bool genAssociated) {
+
+ mlir::MLIRContext *context = module.getContext();
+ // FIXME: Assumed rank arrays not supported yet
+ if (seqTy.hasUnknownShape())
+ return genPlaceholderType(context);
+
+ llvm::SmallVector<mlir::LLVM::DIExpressionElemAttr> ops;
+ auto addOp = [&](unsigned opc, llvm::ArrayRef<uint64_t> vals) {
+ ops.push_back(mlir::LLVM::DIExpressionElemAttr::get(context, opc, vals));
+ };
+
+ addOp(llvm::dwarf::DW_OP_push_object_address, {});
+ addOp(llvm::dwarf::DW_OP_deref, {});
+
+ // dataLocation = *base_addr
+ mlir::LLVM::DIExpressionAttr dataLocation =
+ mlir::LLVM::DIExpressionAttr::get(context, ops);
+ addOp(llvm::dwarf::DW_OP_lit0, {});
+ addOp(llvm::dwarf::DW_OP_ne, {});
+
+ // allocated = associated = (*base_addr != 0)
+ mlir::LLVM::DIExpressionAttr valid =
+ mlir::LLVM::DIExpressionAttr::get(context, ops);
+ mlir::LLVM::DIExpressionAttr associated = genAllocated ? valid : nullptr;
+ mlir::LLVM::DIExpressionAttr allocated = genAssociated ? valid : nullptr;
+ ops.clear();
+
+ llvm::SmallVector<mlir::LLVM::DINodeAttr> elements;
+ mlir::LLVM::DITypeAttr elemTy =
+ convertType(seqTy.getEleTy(), fileAttr, scope, loc);
+ unsigned offset = dimsOffset;
+ const unsigned indexSize = dimsSize / 3;
+ for ([[maybe_unused]] auto _ : seqTy.getShape()) {
+ // For each dimension, find the offset of count and lower bound in the
+ // descriptor and generate the dwarf expression to extract it.
+ // FIXME: If `indexSize` happens to be bigger than address size on the
+ // system then we may have to change 'DW_OP_deref' here.
+ addOp(llvm::dwarf::DW_OP_push_object_address, {});
+ addOp(llvm::dwarf::DW_OP_plus_uconst,
+ {offset + (indexSize * kDimExtentPos)});
+ addOp(llvm::dwarf::DW_OP_deref, {});
+ // count[i] = *(base_addr + offset + (indexSize * kDimExtentPos))
+ // where 'offset' is dimsOffset + (i * dimsSize)
+ mlir::LLVM::DIExpressionAttr countAttr =
+ mlir::LLVM::DIExpressionAttr::get(context, ops);
+ ops.clear();
+
+ addOp(llvm::dwarf::DW_OP_push_object_address, {});
+ addOp(llvm::dwarf::DW_OP_plus_uconst,
+ {offset + (indexSize * kDimLowerBoundPos)});
+ addOp(llvm::dwarf::DW_OP_deref, {});
+ // lower_bound[i] = *(base_addr + offset + (indexSize * kDimLowerBoundPos))
+ mlir::LLVM::DIExpressionAttr lowerAttr =
+ mlir::LLVM::DIExpressionAttr::get(context, ops);
+ ops.clear();
+
+ offset += dimsSize;
+ mlir::LLVM::DISubrangeAttr subrangeTy = mlir::LLVM::DISubrangeAttr::get(
+ context, nullptr, lowerAttr, countAttr, nullptr);
+ elements.push_back(subrangeTy);
+ }
+ 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, dataLocation,
+ /* rank */ nullptr, allocated, associated);
+}
+
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())
@@ -112,6 +242,12 @@ DebugTypeGenerator::convertType(mlir::Type Ty, mlir::LLVM::DIFileAttr fileAttr,
bitWidth * 2, llvm::dwarf::DW_ATE_complex_float);
} else if (auto seqTy = mlir::dyn_cast_or_null<fir::SequenceType>(Ty)) {
return convertSequenceType(seqTy, fileAttr, scope, loc);
+ } else if (auto boxTy = mlir::dyn_cast_or_null<fir::BoxType>(Ty)) {
+ auto elTy = boxTy.getElementType();
+ if (auto seqTy = mlir::dyn_cast_or_null<fir::SequenceType>(elTy))
+ return convertBoxedSequenceType(seqTy, fileAttr, scope, loc, false,
+ false);
+ return genPlaceholderType(context);
} 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 963c919d66825..45e2371af3432 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h
@@ -35,8 +35,20 @@ class DebugTypeGenerator {
mlir::LLVM::DIFileAttr fileAttr,
mlir::LLVM::DIScopeAttr scope,
mlir::Location loc);
+
+ /// The 'genAllocated' is true when we want to generate 'allocated' field
+ /// in the DICompositeType. It is needed for the allocatable arrays.
+ /// Similarly, 'genAssociated' is used with 'pointer' type to generate
+ /// 'associated' field.
+ mlir::LLVM::DITypeAttr
+ convertBoxedSequenceType(fir::SequenceType seqTy,
+ mlir::LLVM::DIFileAttr fileAttr,
+ mlir::LLVM::DIScopeAttr scope, mlir::Location loc,
+ bool genAllocated, bool genAssociated);
mlir::ModuleOp module;
KindMapping kindMapping;
+ size_t dimsSize;
+ size_t dimsOffset;
};
} // namespace fir
diff --git a/flang/test/Integration/debug-assumed-shape-array.f90 b/flang/test/Integration/debug-assumed-shape-array.f90
new file mode 100644
index 0000000000000..7b0801c12dba1
--- /dev/null
+++ b/flang/test/Integration/debug-assumed-shape-array.f90
@@ -0,0 +1,13 @@
+! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck %s
+
+subroutine ff(arr)
+ implicit none
+ integer :: arr(:, :)
+ return arr(1,1)
+end subroutine ff
+
+! CHECK-DAG: !DICompositeType(tag: DW_TAG_array_type{{.*}}elements: ![[ELEMS:[0-9]+]], dataLocation: !DIExpression(DW_OP_push_object_address, DW_OP_deref))
+! CHECK-DAG: ![[ELEMS]] = !{![[ELEM1:[0-9]+]], ![[ELEM2:[0-9]+]]}
+! CHECK-DAG: ![[ELEM1]] = !DISubrange(lowerBound: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 24, DW_OP_deref), upperBound: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 32, DW_OP_deref))
+! CHECK-DAG: ![[ELEM2]] = !DISubrange(lowerBound: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 48, DW_OP_deref), upperBound: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 56, DW_OP_deref))
+
diff --git a/flang/test/Transforms/debug-90683.fir b/flang/test/Transforms/debug-90683.fir
index 9da0e5347d3f8..cc6929c10411f 100644
--- a/flang/test/Transforms/debug-90683.fir
+++ b/flang/test/Transforms/debug-90683.fir
@@ -2,7 +2,7 @@
// This test checks that debug information for fir.real type works ok.
-module attributes {} {
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
func.func @_QPfn1(%arg0: !fir.ref<!fir.complex<8>> {fir.bindc_name = "a"} ) {
%0 = fir.declare %arg0 {uniq_name = "_QFfn1Ea"} : (!fir.ref<!fir.complex<8>>) -> !fir.ref<!fir.complex<8>>
%1 = fir.alloca f32 {bindc_name = "abserror", uniq_name = "_QFfn1Eabserror"}
diff --git a/flang/test/Transforms/debug-assumed-shape-array.fir b/flang/test/Transforms/debug-assumed-shape-array.fir
new file mode 100644
index 0000000000000..00dec9b318c81
--- /dev/null
+++ b/flang/test/Transforms/debug-assumed-shape-array.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<#dlti.dl_entry<i64, dense<64> : vector<2xi64>>, #dlti.dl_entry<!llvm.ptr<272>, dense<64> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<271>, dense<32> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<270>, dense<32> : vector<4xi64>>, #dlti.dl_entry<f128, dense<128> : vector<2xi64>>, #dlti.dl_entry<f80, dense<128> : vector<2xi64>>, #dlti.dl_entry<i128, dense<128> : vector<2xi64>>, #dlti.dl_entry<i8, dense<8> : vector<2xi64>>, #dlti.dl_entry<!llvm.ptr, dense<64> : vector<4xi64>>, #dlti.dl_entry<i1, dense<8> : vector<2xi64>>, #dlti.dl_entry<f16, dense<16> : vector<2xi64>>, #dlti.dl_entry<f64, dense<64> : vector<2xi64>>, #dlti.dl_entry<i32, dense<32> : vector<2xi64>>, #dlti.dl_entry<i16, dense<16> : vector<2xi64>>, #dlti.dl_entry<"dlti.stack_alignment", 128 : i64>, #dlti.dl_entry<"dlti.endianness", "little">>, fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", llvm.data_layout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"} {
+ func.func @ff_(%arg0: !fir.box<!fir.array<?x?xi32>> {fir.bindc_name = "arr"} ) {
+ %0 = fir.undefined !fir.dscope
+ %1 = fircg.ext_declare %arg0 dummy_scope %0 {uniq_name = "_QFffEarr"} : (!fir.box<!fir.array<?x?xi32>>, !fir.dscope) -> !fir.box<!fir.array<?x?xi32>> loc(#loc1)
+ return
+ } loc(#loc2)
+}
+#loc1 = loc("test1.f90":1:1)
+#loc2 = loc("test1.f90":3:16)
+
+// CHECK: #llvm.di_composite_type<tag = DW_TAG_array_type
+// CHECK-SAME: elements = #llvm.di_subrange<lowerBound = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(24), DW_OP_deref]>, upperBound = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(32), DW_OP_deref]>>
+// CHECK-SAME: #llvm.di_subrange<lowerBound = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(48), DW_OP_deref]>, upperBound = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(56), DW_OP_deref]>>
+// CHECK-SAME: dataLocation = <[DW_OP_push_object_address, DW_OP_deref]>>
diff --git a/flang/test/Transforms/debug-complex-1.fir b/flang/test/Transforms/debug-complex-1.fir
index a3cbd767d8a58..cc742d3b183bb 100644
--- a/flang/test/Transforms/debug-complex-1.fir
+++ b/flang/test/Transforms/debug-complex-1.fir
@@ -3,7 +3,7 @@
// check conversion of complex type of different size. Both fir and mlir
// variants are checked.
-module attributes {fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", llvm.target_triple = "native"} {
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
func.func @test1(%x : !fir.complex<4>) -> !fir.complex<8> {
%1 = fir.convert %x : (!fir.complex<4>) -> !fir.complex<8>
return %1 : !fir.complex<8>
diff --git a/flang/test/Transforms/debug-fixed-array-type.fir b/flang/test/Transforms/debug-fixed-array-type.fir
index 401c725411831..d4ed0b9702089 100644
--- a/flang/test/Transforms/debug-fixed-array-type.fir
+++ b/flang/test/Transforms/debug-fixed-array-type.fir
@@ -1,6 +1,6 @@
// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s
-module attributes {} {
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
func.func @_QQmain() attributes {fir.bindc_name = "mn"} {
%c7 = arith.constant 7 : index
%c8 = arith.constant 8 : index
diff --git a/flang/test/Transforms/debug-line-table-existing.fir b/flang/test/Transforms/debug-line-table-existing.fir
index 534278ebc972d..0e006303c8a81 100644
--- a/flang/test/Transforms/debug-line-table-existing.fir
+++ b/flang/test/Transforms/debug-line-table-existing.fir
@@ -3,7 +3,7 @@
// REQUIRES: system-linux
// Test that there are no changes to a function with existed fused loc debug
-module attributes {} {
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
func.func @_QPs1() {
return loc(#loc1)
} loc(#loc2)
diff --git a/flang/test/Transforms/debug-line-table-inc-file.fir b/flang/test/Transforms/debug-line-table-inc-file.fir
index 9370c138fd42f..065039b59c5ae 100644
--- a/flang/test/Transforms/debug-line-table-inc-file.fir
+++ b/flang/test/Transforms/debug-line-table-inc-file.fir
@@ -3,7 +3,7 @@
// REQUIRES: system-linux
// Test for included functions that have a different debug location than the current file
-module attributes {} {
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
func.func @_QPsinc() {
return loc(#loc2)
} loc(#loc1)
@@ -19,7 +19,7 @@ module attributes {} {
#loc4 = loc("/home/user01/llvm-project/build_release/simple.f90":4:3)
#loc5 = loc("/home/user01/llvm-project/build_release/simple.f90":5:1)
-// CHECK: module {
+// CHECK: module
// CHECK: func.func @_QPsinc() {
// CHECK: } loc(#[[FUSED_LOC_INC_FILE:.*]])
// CHECK: func.func @_QQmain() {
diff --git a/flang/test/Transforms/debug-line-table-inc-same-file.fir b/flang/test/Transforms/debug-line-table-inc-same-file.fir
index 4836f2e21dd9d..bcaf449798231 100644
--- a/flang/test/Transforms/debug-line-table-inc-same-file.fir
+++ b/flang/test/Transforms/debug-line-table-inc-same-file.fir
@@ -4,7 +4,7 @@
// Test that there is only one FileAttribute generated for multiple functions
// in the same file.
-module attributes {} {
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
func.func @_QPs1() {
return loc(#loc2)
} loc(#loc1)
diff --git a/flang/test/Transforms/debug-line-table.fir b/flang/test/Transforms/debug-line-table.fir
index 8a72ca2a856a7..d6e54fd1ac467 100644
--- a/flang/test/Transforms/debug-line-table.fir
+++ b/flang/test/Transforms/debug-line-table.fir
@@ -3,7 +3,7 @@
// RUN: fir-opt --add-debug-info="debug-level=LineTablesOnly" --mlir-print-debuginfo %s | FileCheck %s --check-prefix=LINETABLE
// RUN: fir-opt --add-debug-info="is-optimized=true" --mlir-print-debuginfo %s | FileCheck %s --check-prefix=OPT
-module attributes { fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", llvm.data_layout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128", llvm.target_triple = "aarch64-unknown-linux-gnu"} {
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
func.func @_QPsb() {
return loc(#loc_sb)
} loc(#loc_sb)
diff --git a/flang/test/Transforms/debug-module-1.fir b/flang/test/Transforms/debug-module-1.fir
index 822ae01b99aa7..71457d32b1596 100644
--- a/flang/test/Transforms/debug-module-1.fir
+++ b/flang/test/Transforms/debug-module-1.fir
@@ -1,7 +1,7 @@
// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s
-module attributes {} {
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
fir.global @_QMhelperEgli : i32 {
%0 = fir.zero_bits i32
fir.has_value %0 : i32
|
tblah
reviewed
Jun 7, 2024
tblah
reviewed
Jun 7, 2024
| // My testing for a 32-bit shows that base_addr* is correctly given as a | ||
| // 32-bit entity. The index types are 64-bit so I am a bit uncertain how | ||
| // the alignment will effect the calculation of offsets in that case. | ||
|
|
There was a problem hiding this comment.
Maybe this needs an assertion to catch cases where there is padding added to the structure. See ConvertFIRToLLVMPattern::computeBoxSize (FIROpPatterns.cpp) for an example.
There was a problem hiding this comment.
You can query component alignments to the data layout. I think you could have something a bit more generic like (untested):
template<int DescriptorField> std::uint64_t getComponentOffset(const mlir::DataLayout& dl, mlir::Context& context, mlit::Type llvmFieldType) {
mlir::Type previousFieldType = getDescFieldTypeModel<DescriptorField-1>()(context);
std::uint64_t previousOffset = getComponentOffset<DescriptorField-1>(dl, context, previousFieldType);
std::uint64_t offset = previousOffset + dl->getTypeSize(previousFieldType );
std::uint64_t fieldAlignment = dl->getTypeABIAlignment(llvmFieldType);
return llvm::alignTo(offset, fieldAlignment);
}
template<> std::unit64_t getComponentOffset<0>(const mlir::DataLayout& dl, mlir::Context& context) {return 0;}
Then you safely (*) get the info with:
mlir::Type llvmDimsType = getDescFieldTypeModel<kDimsPosInBox>()(context);
std::unit64_t dimOffset = getComponentOffset<kDimsPosInBox>(dl, context, llvmDimsType)
(*) the target/host issue being pushed into the getDescFieldTypeModel<> helper, but not into the code you are adding.
There was a problem hiding this comment.
As @jeanPerier commented, the risk has been pushed to getDescFieldTypeModel<> and not in my code. So I have removed the comments.
jeanPerier
reviewed
Jun 7, 2024
| // My testing for a 32-bit shows that base_addr* is correctly given as a | ||
| // 32-bit entity. The index types are 64-bit so I am a bit uncertain how | ||
| // the alignment will effect the calculation of offsets in that case. | ||
|
|
There was a problem hiding this comment.
You can query component alignments to the data layout. I think you could have something a bit more generic like (untested):
template<int DescriptorField> std::uint64_t getComponentOffset(const mlir::DataLayout& dl, mlir::Context& context, mlit::Type llvmFieldType) {
mlir::Type previousFieldType = getDescFieldTypeModel<DescriptorField-1>()(context);
std::uint64_t previousOffset = getComponentOffset<DescriptorField-1>(dl, context, previousFieldType);
std::uint64_t offset = previousOffset + dl->getTypeSize(previousFieldType );
std::uint64_t fieldAlignment = dl->getTypeABIAlignment(llvmFieldType);
return llvm::alignTo(offset, fieldAlignment);
}
template<> std::unit64_t getComponentOffset<0>(const mlir::DataLayout& dl, mlir::Context& context) {return 0;}
Then you safely (*) get the info with:
mlir::Type llvmDimsType = getDescFieldTypeModel<kDimsPosInBox>()(context);
std::unit64_t dimOffset = getComponentOffset<kDimsPosInBox>(dl, context, llvmDimsType)
(*) the target/host issue being pushed into the getDescFieldTypeModel<> helper, but not into the code you are adding.
|
|
||
| // dims | ||
| dimsSize = | ||
| dl->getTypeSizeInBits(getDescFieldTypeModel<kDimsPosInBox>()(context)); |
There was a problem hiding this comment.
You can directly use getTypeSize() to avoid the /8 I think.
There was a problem hiding this comment.
Thanks for your suggestions. Using the approach results in much cleaner code. I have updated the PR.
tblah
approved these changes
Jun 10, 2024
This PR generates dwarf to extract the information about the arrays
from descriptor. To calculate the offset of the fields in the
descriptor, we are adding up the sizes of the fields above. It seems to
work ok for 64-bit target. Will have to see if some changes are
required for 32-bits targets.
As we use data layout now, some tests needed to be adjusted to have
a dummy data layout to avoid failure.
With this change in place, GDB is able show the assumed shape arrays
correctly.
subroutine ff(n, m, arr)
integer n, m
integer :: arr(:, :)
print *, arr
do i = 1, n
do j = 1, m
arr(j, i) = (i * 5) + j + 10
end do
end do
print *, arr
end subroutine ff
Breakpoint 1, ff (n=4, m=3, arr=...) at test1.f90:13
13 print *, arr
(gdb) p arr
$1 = ((6, 7, 8, 9) (11, 12, 13, 14) (16, 17, 18, 19))
(gdb) ptype arr
type = integer (4,3)
(gdb) c
Continuing.
6 7 8 9 11 12 13 14 16 17 18 19
Add static to getModel() and constexpr to template specializations.
1. Return if data layout is null. 2. Use getComponentOffset to calculate the offset of any field in the descriptor. This was suggested by @jeanPerier.
Lukacma
pushed a commit
to Lukacma/llvm-project
that referenced
this pull request
Jun 12, 2024
This PR generates dwarf to extract the information about the arrays from
descriptor. The DWARF needs the offset of the fields like `lower_bound`
and `extent`. The getComponentOffset has been added to calculate
them which pushes the issue of host and target data size into
getDescFieldTypeModel.
As we use data layout now, some tests needed to be adjusted to have a
dummy data layout to avoid failure.
With this change in place, GDB is able show the assumed shape arrays
correctly.
subroutine ff(n, m, arr)
integer n, m
integer :: arr(:, :)
print *, arr
do i = 1, n
do j = 1, m
arr(j, i) = (i * 5) + j + 10
end do
end do
print *, arr
end subroutine ff
Breakpoint 1, ff (n=4, m=3, arr=...) at test1.f90:13
13 print *, arr
(gdb) p arr
$1 = ((6, 7, 8, 9) (11, 12, 13, 14) (16, 17, 18, 19))
(gdb) ptype arr
type = integer (4,3)
(gdb) c
Continuing.
6 7 8 9 11 12 13 14 16 17 18 19
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR generates dwarf to extract the information about the arrays from descriptor. The DWARF needs the offset of the fields like
lower_boundandextent. To calculate them, we are adding up the sizes of the fields above. It seems to work ok for the 64-bit target. Will have to see if some changes are required for the 32-bits targets.As we use data layout now, some tests needed to be adjusted to have a dummy data layout to avoid failure.
With this change in place, GDB is able show the assumed shape arrays correctly.
subroutine ff(n, m, arr)
integer n, m
integer :: arr(:, :)
print *, arr
do i = 1, n
do j = 1, m
arr(j, i) = (i * 5) + j + 10
end do
end do
print *, arr
end subroutine ff
Breakpoint 1, ff (n=4, m=3, arr=...) at test1.f90:13
13 print *, arr
(gdb) p arr
$1 = ((6, 7, 8, 9) (11, 12, 13, 14) (16, 17, 18, 19))
(gdb) ptype arr
type = integer (4,3)
(gdb) c
Continuing.
6 7 8 9 11 12 13 14 16 17 18 19