[flang][debug] Support derived type components with box types. #109424
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
Our support for derived types uses `getTypeSizeAndAlignment` to calculate the offset of the members. The `fir.box` was not supported in that function. It meant that any member which required descriptor was not supported in the derived type. This PR enhances `getTypeSizeAndAlignment` to support `fir.box`. The implementation uses the following formula to calculate the size. size = offset_of_dims_field + (rank * size_of_dims_field) If an addendum is required then size of its 2 field is also added to the final size. The code to calculate the offset of the descriptor was already present in the `DebugTypeGenerator.cpp`. It has been moved to a separate file so that it can be used in multiple places. There are 2 other changes in this PR: 1. The recID field is used to handle cases where we have a member references its parent type. 2. A type cache is maintained to avoid duplication. It is also needed for circular reference case. Fixes llvm#108001.
abidh
requested review from
tblah,
kiranchandramohan,
jeanPerier,
dpalermo,
pawosm-arm,
jinisusan,
kiranktp and
vzakhari
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-codegen Author: Abid Qadeer (abidh) ChangesOur support for derived types uses This PR enhances The code to calculate the offset of the descriptor was already present in the There are 2 other changes in this PR:
I tested it with the following derived type and the values of all the fields were correct in the debugger. The Fixes #108001. Patch is 21.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/109424.diff 6 Files Affected:
diff --git a/flang/include/flang/Optimizer/CodeGen/DescriptorOffsets.h b/flang/include/flang/Optimizer/CodeGen/DescriptorOffsets.h
new file mode 100644
index 00000000000000..1d62197f38fca3
--- /dev/null
+++ b/flang/include/flang/Optimizer/CodeGen/DescriptorOffsets.h
@@ -0,0 +1,39 @@
+//===-- DescriptorOffsets.h -- offsets of descriptors fields ---*- C++ -*-====//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef OPTIMIZER_DESCRIPTOR_OFFSETS_H
+#define OPTIMIZER_DESCRIPTOR_OFFSETS_H
+
+#include "flang/Optimizer/CodeGen/DescriptorModel.h"
+
+namespace fir {
+
+/// Calculate offset of any field in the descriptor.
+template <int Field>
+static std::uint64_t getDescComponentOffset(const mlir::DataLayout &dl,
+ mlir::MLIRContext *context,
+ mlir::Type fieldType) {
+ static_assert(Field > 0 && Field < 8);
+ mlir::Type previousFieldType = getDescFieldTypeModel<Field - 1>()(context);
+ std::uint64_t previousOffset =
+ getDescComponentOffset<Field - 1>(dl, context, previousFieldType);
+ std::uint64_t offset = previousOffset + dl.getTypeSize(previousFieldType);
+ std::uint64_t fieldAlignment = dl.getTypeABIAlignment(fieldType);
+ return llvm::alignTo(offset, fieldAlignment);
+}
+
+template <>
+std::uint64_t getDescComponentOffset<0>(const mlir::DataLayout &dl,
+ mlir::MLIRContext *context,
+ mlir::Type fieldType) {
+ return 0;
+}
+
+} // namespace fir
+
+#endif // OPTIMIZER_DESCRIPTOR_OFFSETS_H
diff --git a/flang/lib/Optimizer/Dialect/FIRType.cpp b/flang/lib/Optimizer/Dialect/FIRType.cpp
index 05f644654efe1b..694c66e2fc9969 100644
--- a/flang/lib/Optimizer/Dialect/FIRType.cpp
+++ b/flang/lib/Optimizer/Dialect/FIRType.cpp
@@ -13,6 +13,9 @@
#include "flang/Optimizer/Dialect/FIRType.h"
#include "flang/ISO_Fortran_binding_wrapper.h"
#include "flang/Optimizer/Builder/Todo.h"
+#include "flang/Optimizer/CodeGen/DescriptorModel.h"
+#include "flang/Optimizer/CodeGen/DescriptorOffsets.h"
+#include "flang/Optimizer/CodeGen/TypeConverter.h"
#include "flang/Optimizer/Dialect/FIRDialect.h"
#include "flang/Optimizer/Dialect/Support/KindMapping.h"
#include "flang/Tools/PointerModels.h"
@@ -1455,6 +1458,31 @@ fir::getTypeSizeAndAlignment(mlir::Location loc, mlir::Type ty,
compSize *= character.getLen();
return std::pair{compSize, compAlign};
}
+ if (auto boxTy = mlir::dyn_cast<fir::BoxType>(ty)) {
+ mlir::MLIRContext *context = boxTy.getContext();
+ mlir::Type ptrType = getDescFieldTypeModel<kAddrPosInBox>()(context);
+ mlir::Type dimsType = getDescFieldTypeModel<kDimsPosInBox>()(context);
+ std::uint64_t size =
+ getDescComponentOffset<kDimsPosInBox>(dl, context, dimsType);
+ unsigned rank = getBoxRank(ty);
+ if (rank > 0) {
+ std::uint64_t dimsSize = dl.getTypeSize(dimsType);
+ size += (rank * dimsSize);
+ }
+ if (boxHasAddendum(boxTy)) {
+ mlir::Type optType =
+ getExtendedDescFieldTypeModel<kOptTypePtrPosInBox>()(context);
+ mlir::Type rowType =
+ getExtendedDescFieldTypeModel<kOptRowTypePosInBox>()(context);
+ std::uint64_t fieldSize = dl.getTypeSize(optType);
+ std::uint64_t fieldAlignment = dl.getTypeABIAlignment(optType);
+ size += llvm::alignTo(fieldSize, fieldAlignment);
+ fieldSize = dl.getTypeSize(rowType);
+ fieldAlignment = dl.getTypeABIAlignment(rowType);
+ size += llvm::alignTo(fieldSize, fieldAlignment);
+ }
+ return std::pair{size, (unsigned short)dl.getTypeSize(ptrType)};
+ }
return std::nullopt;
}
diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index 1390fae062b934..65784224e07339 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -13,7 +13,7 @@
#define DEBUG_TYPE "flang-debug-type-generator"
#include "DebugTypeGenerator.h"
-#include "flang/Optimizer/CodeGen/DescriptorModel.h"
+#include "flang/Optimizer/CodeGen/DescriptorOffsets.h"
#include "flang/Optimizer/CodeGen/TypeConverter.h"
#include "flang/Optimizer/Support/InternalNames.h"
#include "mlir/Pass/Pass.h"
@@ -59,11 +59,11 @@ DebugTypeGenerator::DebugTypeGenerator(mlir::ModuleOp m,
mlir::Type llvmPtrType = getDescFieldTypeModel<kAddrPosInBox>()(context);
mlir::Type llvmLenType = getDescFieldTypeModel<kElemLenPosInBox>()(context);
dimsOffset =
- getComponentOffset<kDimsPosInBox>(*dataLayout, context, llvmDimsType);
+ getDescComponentOffset<kDimsPosInBox>(*dataLayout, context, llvmDimsType);
dimsSize = dataLayout->getTypeSize(llvmDimsType);
ptrSize = dataLayout->getTypeSize(llvmPtrType);
- lenOffset =
- getComponentOffset<kElemLenPosInBox>(*dataLayout, context, llvmLenType);
+ lenOffset = getDescComponentOffset<kElemLenPosInBox>(*dataLayout, context,
+ llvmLenType);
}
static mlir::LLVM::DITypeAttr genBasicType(mlir::MLIRContext *context,
@@ -163,7 +163,24 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertBoxedSequenceType(
mlir::LLVM::DITypeAttr DebugTypeGenerator::convertRecordType(
fir::RecordType Ty, mlir::LLVM::DIFileAttr fileAttr,
mlir::LLVM::DIScopeAttr scope, fir::cg::XDeclareOp declOp) {
+ // Check if this type has already been converted.
+ auto iter = typeCache.find(Ty);
+ if (iter != typeCache.end())
+ return iter->second;
+
+ llvm::SmallVector<mlir::LLVM::DINodeAttr> elements;
mlir::MLIRContext *context = module.getContext();
+ auto recId = mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
+ // Generate a place holder TypeAttr which will be used if a member
+ // references the parent type.
+ auto comAttr = mlir::LLVM::DICompositeTypeAttr::get(
+ context, recId, /*isRecSelf=*/true, llvm::dwarf::DW_TAG_structure_type,
+ mlir::StringAttr::get(context, ""), fileAttr, /*line=*/0, scope,
+ /*baseType=*/nullptr, mlir::LLVM::DIFlags::Zero, /*sizeInBits=*/0,
+ /*alignInBits=*/0, elements, /*dataLocation=*/nullptr, /*rank=*/nullptr,
+ /*allocated=*/nullptr, /*associated=*/nullptr);
+ typeCache[Ty] = comAttr;
+
auto result = fir::NameUniquer::deconstruct(Ty.getName());
if (result.first != fir::NameUniquer::NameKind::DERIVED_TYPE)
return genPlaceholderType(context);
@@ -171,7 +188,6 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertRecordType(
fir::TypeInfoOp tiOp = symbolTable->lookup<fir::TypeInfoOp>(Ty.getName());
unsigned line = (tiOp) ? getLineFromLoc(tiOp.getLoc()) : 1;
- llvm::SmallVector<mlir::LLVM::DINodeAttr> elements;
std::uint64_t offset = 0;
for (auto [fieldName, fieldTy] : Ty.getTypeList()) {
auto result = fir::getTypeSizeAndAlignment(module.getLoc(), fieldTy,
@@ -195,12 +211,14 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertRecordType(
offset += llvm::alignTo(byteSize, byteAlign);
}
- return mlir::LLVM::DICompositeTypeAttr::get(
- context, llvm::dwarf::DW_TAG_structure_type,
+ auto finalAttr = mlir::LLVM::DICompositeTypeAttr::get(
+ context, recId, /*isRecSelf=*/false, llvm::dwarf::DW_TAG_structure_type,
mlir::StringAttr::get(context, result.second.name), fileAttr, line, scope,
/*baseType=*/nullptr, mlir::LLVM::DIFlags::Zero, offset * 8,
/*alignInBits=*/0, elements, /*dataLocation=*/nullptr, /*rank=*/nullptr,
/*allocated=*/nullptr, /*associated=*/nullptr);
+ typeCache[Ty] = finalAttr;
+ return finalAttr;
}
mlir::LLVM::DITypeAttr DebugTypeGenerator::convertSequenceType(
diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h
index e3220f18958df2..104f3591d5ba8a 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h
@@ -72,6 +72,7 @@ class DebugTypeGenerator {
std::uint64_t dimsOffset;
std::uint64_t ptrSize;
std::uint64_t lenOffset;
+ llvm::DenseMap<mlir::Type, mlir::LLVM::DITypeAttr> typeCache;
};
} // namespace fir
diff --git a/flang/test/Integration/debug-cyclic-derived-type.f90 b/flang/test/Integration/debug-cyclic-derived-type.f90
index 03e06336a6e084..0325f62a0a9b4f 100644
--- a/flang/test/Integration/debug-cyclic-derived-type.f90
+++ b/flang/test/Integration/debug-cyclic-derived-type.f90
@@ -11,5 +11,11 @@ module m
type(t2) :: v3
end module
-! CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "t1"{{.*}})
-! CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "t2"{{.*}})
+! CHECK-DAG: ![[T1:[0-9]+]] = {{.*}}!DICompositeType(tag: DW_TAG_structure_type, name: "t1"{{.*}}elements: ![[T1_ELEMS:[0-9]+]])
+! CHECK-DAG: ![[T1_ELEMS]] = !{![[T1_ELEM1:[0-9]+]]}
+! CHECK-DAG: ![[T1_ELEM1]] = !DIDerivedType(tag: DW_TAG_member, name: "p", baseType: ![[T2P:[0-9]+]]{{.*}})
+! CHECK-DAG: ![[T2P]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[T2:[0-9]+]]{{.*}})
+
+! CHECK-DAG: ![[T2]] = {{.*}}!DICompositeType(tag: DW_TAG_structure_type, name: "t2"{{.*}}elements: ![[T2_ELEMS:[0-9]+]])
+! CHECK-DAG: ![[T2_ELEMS]] = !{![[T2_ELEM1:[0-9]+]]}
+! CHECK-DAG: ![[T2_ELEM1]] = !DIDerivedType(tag: DW_TAG_member, name: "v1", baseType: ![[T1]]{{.*}})
\ No newline at end of file
diff --git a/flang/test/Transforms/debug-derived-type-1.fir b/flang/test/Transforms/debug-derived-type-1.fir
index e453db6ae6fbb7..26f7017f5f5a38 100644
--- a/flang/test/Transforms/debug-derived-type-1.fir
+++ b/flang/test/Transforms/debug-derived-type-1.fir
@@ -12,12 +12,18 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<!llvm.ptr<272>, d
%0 = fir.zero_bits !fir.type<_QMt1Tt_t1{age:i32,points:!fir.array<3x!fir.complex<4>>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}>
fir.has_value %0 : !fir.type<_QMt1Tt_t1{age:i32,points:!fir.array<3x!fir.complex<4>>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}>
} loc(#loc6)
+ fir.global @_QMtest_1Exyz : !fir.type<_QMtest_1Tsometype{m_array:!fir.array<3xi32>,m_vt1:!fir.box<!fir.heap<!fir.type<_QMtest_1Tt1{name:!fir.char<1,20>,height:f32}>>>,v2:i32,m_alloc:!fir.box<!fir.heap<!fir.array<?xi32>>>,v3:i32,m_first:!fir.box<!fir.heap<!fir.char<1,?>>>,v4:i32,m_p1:!fir.box<!fir.ptr<i32>>,v5:i32,m_p2:!fir.box<!fir.ptr<i32>>,v6:i32,m_p3:!fir.box<!fir.ptr<!fir.array<?xi32>>>,v7:i32}> {
+ %0 = fir.zero_bits !fir.type<_QMtest_1Tsometype{m_array:!fir.array<3xi32>,m_vt1:!fir.box<!fir.heap<!fir.type<_QMtest_1Tt1{name:!fir.char<1,20>,height:f32}>>>,v2:i32,m_alloc:!fir.box<!fir.heap<!fir.array<?xi32>>>,v3:i32,m_first:!fir.box<!fir.heap<!fir.char<1,?>>>,v4:i32,m_p1:!fir.box<!fir.ptr<i32>>,v5:i32,m_p2:!fir.box<!fir.ptr<i32>>,v6:i32,m_p3:!fir.box<!fir.ptr<!fir.array<?xi32>>>,v7:i32}>
+ fir.has_value %0 : !fir.type<_QMtest_1Tsometype{m_array:!fir.array<3xi32>,m_vt1:!fir.box<!fir.heap<!fir.type<_QMtest_1Tt1{name:!fir.char<1,20>,height:f32}>>>,v2:i32,m_alloc:!fir.box<!fir.heap<!fir.array<?xi32>>>,v3:i32,m_first:!fir.box<!fir.heap<!fir.char<1,?>>>,v4:i32,m_p1:!fir.box<!fir.ptr<i32>>,v5:i32,m_p2:!fir.box<!fir.ptr<i32>>,v6:i32,m_p3:!fir.box<!fir.ptr<!fir.array<?xi32>>>,v7:i32}>
+ } loc(#loc12)
fir.type_info @_QMt1Tt_t1 noinit nodestroy nofinal : !fir.type<_QMt1Tt_t1{age:i32,points:!fir.array<3x!fir.complex<4>>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> loc(#loc7)
fir.type_info @_QMm_employeeTt_address noinit nodestroy nofinal : !fir.type<_QMm_employeeTt_address{house_number:i32}> loc(#loc1)
fir.type_info @_QMm_employeeTt_person noinit nodestroy nofinal extends !fir.type<_QMm_employeeTt_address{house_number:i32}> : !fir.type<_QMm_employeeTt_person{t_address:!fir.type<_QMm_employeeTt_address{house_number:i32}>,name:!fir.char<1,20>}> loc(#loc2)
fir.type_info @_QMm_employeeTt_date noinit nodestroy nofinal : !fir.type<_QMm_employeeTt_date{year:i32,month:i32,day:i32}> loc(#loc3)
fir.type_info @_QMm_employeeTt_employee noinit nodestroy nofinal extends !fir.type<_QMm_employeeTt_person{t_address:!fir.type<_QMm_employeeTt_address{house_number:i32}>,name:!fir.char<1,20>}> : !fir.type<_QMm_employeeTt_employee{t_person:!fir.type<_QMm_employeeTt_person{t_address:!fir.type<_QMm_employeeTt_address{house_number:i32}>,name:!fir.char<1,20>}>,hired_date:!fir.type<_QMm_employeeTt_date{year:i32,month:i32,day:i32}>,monthly_salary:f32}> loc(#loc4)
fir.type_info @_QFTt_pair noinit nodestroy nofinal : !fir.type<_QFTt_pair{i:i64,x:f64}> loc(#loc8)
+ fir.type_info @_QMtest_1Tt1 noinit nodestroy nofinal : !fir.type<_QMtest_1Tt1{name:!fir.char<1,20>,height:f32}> loc(#loc11)
+ fir.type_info @_QMtest_1Tsometype nofinal : !fir.type<_QMtest_1Tsometype{m_array:!fir.array<3xi32>,m_vt1:!fir.box<!fir.heap<!fir.type<_QMtest_1Tt1{name:!fir.char<1,20>,height:f32}>>>,v2:i32,m_alloc:!fir.box<!fir.heap<!fir.array<?xi32>>>,v3:i32,m_first:!fir.box<!fir.heap<!fir.char<1,?>>>,v4:i32,m_p1:!fir.box<!fir.ptr<i32>>,v5:i32,m_p2:!fir.box<!fir.ptr<i32>>,v6:i32,m_p3:!fir.box<!fir.ptr<!fir.array<?xi32>>>,v7:i32}> loc(#loc12)
func.func @_QQmain() attributes {fir.bindc_name = "test"} {
%1 = fir.alloca !fir.type<_QFTt_pair{i:i64,x:f64}> {bindc_name = "pair", uniq_name = "_QFEpair"}
%2 = fircg.ext_declare %1 {uniq_name = "_QFEpair"} : (!fir.ref<!fir.type<_QFTt_pair{i:i64,x:f64}>>) -> !fir.ref<!fir.type<_QFTt_pair{i:i64,x:f64}>> loc(#loc9)
@@ -34,6 +40,8 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<!llvm.ptr<272>, d
#loc8 = loc("derived1.f90":85:3)
#loc9 = loc("derived1.f90":77:3)
#loc10 = loc("derived1.f90":75:3)
+#loc11 = loc("derived1.f90":95:3)
+#loc12 = loc("derived1.f90":105:3)
// CHECK-DAG: #[[INT_TY:.*]] = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "integer", sizeInBits = 32, encoding = DW_ATE_signed>
@@ -47,27 +55,42 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<!llvm.ptr<272>, d
// CHECK-DAG: #[[MOD:.*]] = #llvm.di_module<{{.*}}name = "m_employee"{{.*}}>
// CHECK-DAG: #[[MOD1:.*]] = #llvm.di_module<{{.*}}name = "t1"{{.*}}>
// CHECK-DAG: #[[ELMA1:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "house_number", baseType = #[[INT_TY]], sizeInBits = 32, alignInBits = 32>
-// CHECK-DAG: #[[ADDR:.*]] = #llvm.di_composite_type<tag = DW_TAG_structure_type, name = "t_address"{{.*}}line = 24, scope = #[[MOD]], sizeInBits = 32, elements = #[[ELMA1]]>
+// CHECK-DAG: #[[ADDR:.*]] = #llvm.di_composite_type<{{.*}}tag = DW_TAG_structure_type, name = "t_address"{{.*}}line = 24, scope = #[[MOD]], sizeInBits = 32, elements = #[[ELMA1]]>
// CHECK-DAG: #[[ELMD1:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "year", baseType = #[[INT_TY]], sizeInBits = 32, alignInBits = 32>
// CHECK-DAG: #[[ELMD2:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "month", baseType = #[[INT_TY]], sizeInBits = 32, alignInBits = 32, offsetInBits = 32>
// CHECK-DAG: #[[ELMD3:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "day", baseType = #[[INT_TY]], sizeInBits = 32, alignInBits = 32, offsetInBits = 64>
-// CHECK-DAG: #[[DATE:.*]] = #llvm.di_composite_type<tag = DW_TAG_structure_type, name = "t_date", file = #di_file, line = 17, scope = #[[MOD]], sizeInBits = 96, elements = #[[ELMD1]], #[[ELMD2]], #[[ELMD3]]>
+// CHECK-DAG: #[[DATE:.*]] = #llvm.di_composite_type<{{.*}}tag = DW_TAG_structure_type, name = "t_date", file = #di_file, line = 17, scope = #[[MOD]], sizeInBits = 96, elements = #[[ELMD1]], #[[ELMD2]], #[[ELMD3]]>
// CHECK-DAG: #[[ELMP1:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "t_address", baseType = #[[ADDR]], sizeInBits = 32, alignInBits = 32>
// CHECK-DAG: #[[ELMP2:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "name", baseType = #[[STR_TY]], sizeInBits = 160, alignInBits = 8, offsetInBits = 32>
-// CHECK-DAG: #[[PERS:.*]] = #llvm.di_composite_type<tag = DW_TAG_structure_type, name = "t_person"{{.*}}line = 35, scope = #[[MOD]], sizeInBits = 192, elements = #[[ELMP1]], #[[ELMP2]]>
+// CHECK-DAG: #[[PERS:.*]] = #llvm.di_composite_type<{{.*}}tag = DW_TAG_structure_type, name = "t_person"{{.*}}line = 35, scope = #[[MOD]], sizeInBits = 192, elements = #[[ELMP1]], #[[ELMP2]]>
// CHECK-DAG: #[[ELME1:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "t_person", baseType = #[[PERS]], sizeInBits = 192, alignInBits = 32>
// CHECK-DAG: #[[ELME2:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "hired_date", baseType = #[[DATE]], sizeInBits = 96, alignInBits = 32, offsetInBits = 192>
// CHECK-DAG: #[[ELME3:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "monthly_salary", baseType = #[[REAL4_TY]], sizeInBits = 32, alignInBits = 32, offsetInBits = 288>
-// CHECK-DAG: #[[EMP:.*]] = #llvm.di_composite_type<tag = DW_TAG_structure_type, name = "t_employee"{{.*}}line = 46, scope = #[[MOD]], sizeInBits = 320, elements = #[[ELME1]], #[[ELME2]], #[[ELME3]]>
+// CHECK-DAG: #[[EMP:.*]] = #llvm.di_composite_type<{{.*}}tag = DW_TAG_structure_type, name = "t_employee"{{.*}}line = 46, scope = #[[MOD]], sizeInBits = 320, elements = #[[ELME1]], #[[ELME2]], #[[ELME3]]>
// CHECK-DAG: #[[ELM1:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "age", baseType = #[[INT_TY]], sizeInBits = 32, alignInBits = 32>
// CHECK-DAG: #[[ELM2:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "points", baseType = #[[CMX_ARR]], sizeInBits = 192, alignInBits = 32, offsetInBits = 32>
// CHECK-DAG: #[[ELM3:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "cond", baseType = #[[LOG_TY]], sizeInBits = 8, alignInBits = 8, offsetInBits = 224>
// CHECK-DAG: #[[ELM4:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "name", baseType = #[[STR_TY]], sizeInBits = 160, alignInBits = 8, offsetInBits = 232>
// CHECK-DAG: #[[ELM5:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "ratio", baseType = #[[REAL8_TY]], sizeInBits = 64, alignInBits = 64, offsetInBits = 448>
-// CHECK-DAG: #llvm.di_composite_type<tag = DW_TAG_structure_type, name = "t_t1"{{.*}}, line = 70, scope = #[[MOD1]], sizeInBits = 512, elements = #[[ELM1]], #[[ELM2]], #[[ELM3]], #[[ELM4]], #[[ELM5]]>
+// CHECK-DAG: #llvm.di_composite_type<{{.*}}tag = DW_TAG_structure_type, name = "t_t1"{{.*}}, line = 70, scope = #[[MOD1]], sizeInBits = 512, elements = #[[ELM1]], #[[ELM2]], #[[ELM3]], #[[ELM4]], #[[ELM5]]>
// CHECK-DAG: #[[SP:.*]] = #llvm.di_subprogram
// CHECK-DAG: #[[ELML1:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "i", baseType = #[[INT8_TY]], sizeInBits = 64, alignInBits = 64>
// CHECK-DAG: #[[ELML2:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "x", baseType = #[[REAL8_TY]], sizeInBits = 64, alignInBits = 64, offsetInBits = 64>
-// CHECK-DAG: #llvm.di_composite_type<tag = DW_TAG_structure_type, name = "t_pair"{{.*}}line = 85, scope = #di_subprogram, sizeInBits = 128, elements = #[[ELML1]], #[[ELML2]]>
+// CHECK-DAG: #llvm.di_composite_type<{{.*}}tag = DW_TAG_structure_type, name = "t_pair"{{.*}}line = 85, scope = #di_subprogram, sizeInBits = 128, elements = #[[ELML1]], #[[ELML2]]>
+
+// CHECK-DAG: #[[E1:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "m_array", baseType = #{{.*}}, sizeInBits = 96, alignInBits = 32>
+// CHECK-DAG: #[[E2:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "m_vt1", baseType = #{{.*}}, sizeInBits = 320, alignInBits = 64, offsetInBits = 128>
+// CHECK-DAG: #[[E3:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "v2", baseType = #{{.*}}, sizeInBits = 32, alignInBits = 32, offsetInBits = 448>
+// CHECK-DAG: #[[E4:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "m_alloc", baseType = #{{.*}}, sizeInBits = 384, alignInBits = 64, offsetInBits = 512>
+// CHECK-DAG: #[[E5:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "v3", baseType = #{{.*}}, sizeInBits = 32, alignInBits = 32, offsetInBits = 896>
+// CHECK-DAG: #[[E6:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "m_first", baseType = #{{.*}}, sizeInBits = 192, alignInBits = 64, offsetInBits = 960>
+// CHECK-DAG: #[[E7:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "v4", baseType = #{{.*}}, sizeInBits = 32, alignInBits = 32, offsetInBits = 1152>
+// CHECK-DAG: #[[E8:.*]] = ...
[truncated]
|
pawosm-arm
reviewed
Sep 20, 2024
pawosm-arm
approved these changes
Sep 20, 2024
pawosm-arm
reviewed
Sep 23, 2024
jeanPerier
reviewed
Sep 23, 2024
| @@ -13,6 +13,9 @@ | |||
| #include "flang/Optimizer/Dialect/FIRType.h" | |||
| #include "flang/ISO_Fortran_binding_wrapper.h" | |||
| #include "flang/Optimizer/Builder/Todo.h" | |||
| #include "flang/Optimizer/CodeGen/DescriptorModel.h" | |||
| #include "flang/Optimizer/CodeGen/DescriptorOffsets.h" | |||
| #include "flang/Optimizer/CodeGen/TypeConverter.h" | |||
There was a problem hiding this comment.
So far I was rather against handling fir.box<> into getTypeSizeAndAlignment because to me it is meant to be an abstract type as much possible (could be implemented as a CFI descriptor, or some descriptor compatible with existing compilers if we wanted). Instead, I was more in favor of translating type to LLVM and using getTypeSizeAndAlignment at that level. That way, the codegen of abstract FIR types is centralized in the type codegen and needs not to be spilled in other places.
Now I understand this is inconvenient given there are several places that needs to know about the size at different stage of the compilation.
But as far as debugging is concerned, I would still be in favor of calling translating types to LLVM and calling getTypeSizeAndAlignment with that.
Otherwise, we at least need to centralize all low level descriptor related tools in some flang/Optimizer/Dialect/Support/DescriptorModel.h file. The Dialect library should not depend upon Codegen.
There was a problem hiding this comment.
Thanks for your comments. I have updated the PR and it does not use getTypeSizeAndAlignment to get the size for any type. We use LLVMTypeConverter to convert fieldTy into LLVM type and then use DataLayout to get the size. It all seems to works ok. My concern was whether it is safe to call DataLayout::getTypeSize on any type that is returned by LLVMTypeConverter::convertType. I did not see any problem with my testing but would be happy to add any type specific check.
| mlir::Type dimsType = getDescFieldTypeModel<kDimsPosInBox>()(context); | ||
| std::uint64_t size = | ||
| getDescComponentOffset<kDimsPosInBox>(dl, context, dimsType); | ||
| unsigned rank = getBoxRank(ty); |
There was a problem hiding this comment.
What about assumed-ranks, the descriptor size is not a compile time constant? They cannot be used as derived type components, but this generic helper would need to handle them or bail correctly.
Use LLVMTypeConverter to convert the fir.box into an mlir::Type and then get its size from DataLayout.
jeanPerier
approved these changes
Sep 25, 2024
| std::uint64_t offset = 0; | ||
| LLVMTypeConverter llvmTypeConverter(module, false, false, *dataLayout); |
There was a problem hiding this comment.
This is likely not a trivial class/ctor, and I would not be surprised if it did caching somewhere. Probably best to keep it at the same level as the dataLayout.
This helps avoid its instantiation for every RecordType.
jeanPerier
approved these changes
Sep 25, 2024
This commit fixes an issue that can cause an assertion to fail in some circumstances at https://github.com/llvm/llvm-project/blob/26029d77a57cb4aaa1479064109e985a90d0edd8/mlir/lib/Target/LLVMIR/DebugTranslation.cpp#L270. The issue relates to the handling of recursive debug type in mlir. See the discussion at the end of follow PR for more details. llvm#106571 Problem could be explained with the following example code: type t2 type(t1), pointer :: p1 end type type t1 type(t2), pointer :: p2 end type In the description below, type_self means a temporary type that is generated as a place holder while the members of that type are being processed. If we process t1 first then we will have the following structure after it has been processed. t1 -> t2 -> t1_self This is because when we started processing t2, we did not have the complete t1 but its place holder t1_self. Now if some entity requires t2, we will already have that in cache and will return it. But this t2 refers to t1_self and not to t1. In mlir handling, only those types are allowed to have _self reference which are wrapped by entity whose reference it contains. So t1 -> t2 -> t1_self is ok because the t1_self reference can be resolved by the outer t1. But standalone t2 is not because there will be no way to resolve it. Please see DebugTranslation::translateRecursive for details on how mlir handles recursive types. The fix is not to cache the type that will fail if used standalone.
|
While testing, I noticed an assertion failure at the following location. I got the following advice when I asked how best to handle the issue with the author of the code. I have implemented this in this commit by not caching a type if we know that its standalone use could cause a problem. This will result a new type attribute to be generated for that type if it is used standalone. |
xgupta
pushed a commit
to xgupta/llvm-project
that referenced
this pull request
Oct 4, 2024
…109424) Our support for derived types uses `getTypeSizeAndAlignment` to calculate the offset of the members. The `fir.box` was not supported in that function. It meant that any member which required descriptor was not supported in the derived type. We convert the type into an llvm type and then use the DataLayout to calculate the size/offset of a member. There is no dependency on `getTypeSizeAndAlignment` to get the size of the types. There are 2 other changes in this PR: 1. The `recID` field is used to handle cases where we have a member references its parent type. 2. A type cache is maintained to avoid duplication. It is also needed for circular reference case. Fixes llvm#108001.
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.
Our support for derived types uses
getTypeSizeAndAlignmentto calculate the offset of the members. Thefir.boxwas not supported in that function. It meant that any member which required descriptor was not supported in the derived type.This PR enhances
getTypeSizeAndAlignmentto supportfir.box. The implementation uses the following formula to calculate the size.size = offset_of_dims_field + (rank * size_of_dims_field)
If an addendum is required then size of its 2 field is also added to the final size.
The code to calculate the offset of the descriptor was already present in the
DebugTypeGenerator.cpp. It has been moved to a separate file so that it can be used in multiple places.There are 2 other changes in this PR:
recIDfield is used to handle cases where we have a member references its parent type.I tested it with the following derived type and the values of all the fields were correct in the debugger. The
debug-cyclic-derived-type.f90which tests the cyclic dependency has been updated and works as expected.Fixes #108001.