Skip to content

Commit d26a646

Browse files
jeanPerierjeanPerier
authored
[flang] Do not leave length parameters uninitialized in descriptor addendums (#81858)
Descriptor addendum have a field to hold length parameters (currently only one). This field is currently never used because flang does not lowered derived types with length parameters. However, leaving it uninitialized is causing bugs in code like gFTL where the code is trying to sort POINTERs (see [1]). More precisely, it is an issue when two pointers should compare equal (same base address), because the uninitialized values in the addendum may differ depending on the "stack history" and optimization level. Always initialized the length parameters field in the addendum to zero. [1]: https://github.com/Goddard-Fortran-Ecosystem/gFTL/blob/dc93a5fc2fe2b487590580d9f0a17d77ca3fe5c1/include/v1/templates/set_impl.inc#L312 The type being transferred to an integer array may look like: ``` TYPE :: localwrapper TYPE(T), POINTER :: item END TYPE localwrapper ``` Which in flang case ends-up transferring a descriptor to an integer array, the code in gFTL later compare the integer arrays. This logic is used when building set data structures in gFTL.
1 parent 42b5037 commit d26a646

File tree

7 files changed

+31
-14
lines changed

7 files changed

+31
-14
lines changed

‎flang/include/flang/Runtime/descriptor.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ class DescriptorAddendum {
9999
public:
100100
explicit RT_API_ATTRS DescriptorAddendum(
101101
const typeInfo::DerivedType *dt = nullptr)
102-
: derivedType_{dt} {}
102+
: derivedType_{dt}, len_{0} {}
103103
RT_API_ATTRS DescriptorAddendum &operator=(const DescriptorAddendum &);
104104

105105
const RT_API_ATTRS typeInfo::DerivedType *derivedType() const {

‎flang/lib/Optimizer/CodeGen/CodeGen.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,9 @@ static unsigned getTypeDescFieldId(mlir::Type ty) {
147147
auto isArray = fir::dyn_cast_ptrOrBoxEleTy(ty).isa<fir::SequenceType>();
148148
return isArray ? kOptTypePtrPosInBox : kDimsPosInBox;
149149
}
150+
static unsigned getLenParamFieldId(mlir::Type ty) {
151+
return getTypeDescFieldId(ty) + 1;
152+
}
150153

151154
namespace {
152155
/// FIR conversion pattern template
@@ -1583,6 +1586,14 @@ struct EmboxCommonConversion : public FIROpConversion<OP> {
15831586
descriptor =
15841587
insertField(rewriter, loc, descriptor, {typeDescFieldId}, typeDesc,
15851588
/*bitCast=*/true);
1589+
// Always initialize the length parameter field to zero to avoid issues
1590+
// with uninitialized values in Fortran code trying to compare physical
1591+
// representation of derived types with pointer/allocatable components.
1592+
// This has been seen in hashing algorithms using TRANSFER.
1593+
mlir::Value zero =
1594+
genConstantIndex(loc, rewriter.getI64Type(), rewriter, 0);
1595+
descriptor = insertField(rewriter, loc, descriptor,
1596+
{getLenParamFieldId(boxTy), 0}, zero);
15861597
}
15871598
return descriptor;
15881599
}

‎flang/test/Fir/ignore-missing-type-descriptor.fir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,5 @@ func.func @test_embox(%addr: !fir.ref<!some_freestyle_type>) {
1818
// CHECK-SAME: ptr %[[ADDR:.*]])
1919
// CHECK: insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }
2020
// CHECK-SAME: { ptr undef, i64 ptrtoint (ptr getelementptr (%some_not_mangled_type, ptr null, i32 1) to i64),
21-
// CHECK-SAME: i32 20180515, i8 0, i8 42, i8 0, i8 1, ptr null, [1 x i64] undef },
21+
// CHECK-SAME: i32 20180515, i8 0, i8 42, i8 0, i8 1, ptr null, [1 x i64] zeroinitializer },
2222
// CHECK-SAME: ptr %[[ADDR]], 0

‎flang/test/Fir/polymorphic.fir

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ func.func @_QMpolymorphic_testPtest_allocate_unlimited_polymorphic_non_derived()
1313
// CHECK-LABEL: define void @_QMpolymorphic_testPtest_allocate_unlimited_polymorphic_non_derived(){{.*}}{
1414
// CHECK: %[[MEM:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }
1515
// CHECK: %[[DESC:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }, i64 1
16-
// CHECK: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } { ptr null, i64 0, i32 20180515, i8 0, i8 -1, i8 1, i8 1, ptr null, [1 x i64] undef }, ptr %[[MEM]]
16+
// CHECK: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } { ptr null, i64 0, i32 20180515, i8 0, i8 -1, i8 1, i8 1, ptr null, [1 x i64] zeroinitializer }, ptr %[[MEM]]
1717
// CHECK: %[[LOADED:.*]] = load { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }, ptr %[[MEM]], align 8
1818
// CHECK: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[LOADED]], ptr %[[DESC]]
1919
// CHECK: ret void
@@ -100,7 +100,8 @@ func.func @_QMunlimitedPsub1(%arg0: !fir.class<!fir.array<?xnone>> {fir.bindc_na
100100
// CHECK: %[[TYPE_CODE_TRUNC:.*]] = trunc i32 %[[TYPE_CODE]] to i8
101101
// CHECK: %{{.*}} = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %{{.*}}, i8 %[[TYPE_CODE_TRUNC]], 4
102102
// CHECK: %{{.*}} = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %{{.*}}, ptr %[[TYPE_DESC]], 7
103-
// CHECK: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %24, ptr %[[BOX]]
103+
// CHECK: %{{.*}} = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %{{.*}}, i64 0, 8, 0
104+
// CHECK: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %{{.*}}, ptr %[[BOX]]
104105

105106

106107
fir.global internal @_QFEx : !fir.class<!fir.ptr<!fir.array<?xnone>>> {
@@ -154,7 +155,7 @@ func.func @_QQmain() {
154155
// CHECK-LABEL: define void @_QQmain(){{.*}}{
155156
// CHECK: %[[CLASS_NONE:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }
156157
// CHECK: %[[DESC:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }, i64 1
157-
// CHECK: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } { ptr @_QMmod1Ea, i64 ptrtoint (ptr getelementptr (%_QMmod1TtK2, ptr null, i32 1) to i64), i32 20180515, i8 0, i8 42, i8 1, i8 1, ptr @_QMmod1E.dt.t.2, [1 x i64] undef }, ptr %[[CLASS_NONE]], align 8
158+
// CHECK: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } { ptr @_QMmod1Ea, i64 ptrtoint (ptr getelementptr (%_QMmod1TtK2, ptr null, i32 1) to i64), i32 20180515, i8 0, i8 42, i8 1, i8 1, ptr @_QMmod1E.dt.t.2, [1 x i64] zeroinitializer }, ptr %[[CLASS_NONE]], align 8
158159
// CHECK: %[[LOAD:.*]] = load { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }, ptr %[[CLASS_NONE]]
159160
// CHECK: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[LOAD]], ptr %[[DESC]]
160161
// CHECK: call void @_QMmod1Psub1(ptr %[[DESC]])
@@ -194,4 +195,4 @@ func.func @_QQembox_input_type(%arg0 : !fir.ref<!fir.type<_QMmod1Tp2{v:!fir.arra
194195
}
195196

196197
// CHECK-LABEL: define void @_QQembox_input_type
197-
// CHECK: %{{.*}} = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } { ptr undef, i64 ptrtoint (ptr getelementptr (%_QMmod1Tp2, ptr null, i32 1) to i64), i32 20180515, i8 0, i8 42, i8 0, i8 1, ptr @_QMmod1E.dt.p2, [1 x i64] undef }, ptr %{{.*}}, 0
198+
// CHECK: %{{.*}} = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } { ptr undef, i64 ptrtoint (ptr getelementptr (%_QMmod1Tp2, ptr null, i32 1) to i64), i32 20180515, i8 0, i8 42, i8 0, i8 1, ptr @_QMmod1E.dt.p2, [1 x i64] zeroinitializer }, ptr %{{.*}}, 0

‎flang/test/Fir/tbaa.fir

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ module {
6767
// CHECK: %[[VAL_36:.*]] = llvm.mlir.constant(1 : i32) : i32
6868
// CHECK: %[[VAL_37:.*]] = llvm.trunc %[[VAL_36]] : i32 to i8
6969
// CHECK: %[[VAL_38:.*]] = llvm.insertvalue %[[VAL_37]], %[[VAL_35]][6] : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, ptr, array<1 x i64>)>
70-
// CHECK: %[[VAL_40:.*]] = llvm.insertvalue %[[VAL_19]], %[[VAL_38]][7] : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, ptr, array<1 x i64>)>
70+
// CHECK: %[[VAL_40B:.*]] = llvm.insertvalue %[[VAL_19]], %[[VAL_38]][7] : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, ptr, array<1 x i64>)>
71+
// CHECK: %[[VAL_40:.*]] = llvm.insertvalue %{{.*}}, %[[VAL_40B]][8, 0] : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, ptr, array<1 x i64>)>
7172
// CHECK: %[[VAL_42:.*]] = llvm.insertvalue %[[VAL_14]], %[[VAL_40]][0] : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, ptr, array<1 x i64>)>
7273
// CHECK: llvm.store %[[VAL_42]], %[[VAL_2]] {tbaa = [#[[$BOXT]]]} : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, ptr, array<1 x i64>)>, !llvm.ptr
7374
// CHECK: %[[VAL_43:.*]] = llvm.getelementptr %[[VAL_2]][0, 4] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, ptr, array<1 x i64>)>
@@ -166,7 +167,8 @@ module {
166167
// CHECK: %[[VAL_40:.*]] = llvm.mlir.constant(1 : i32) : i32
167168
// CHECK: %[[VAL_41:.*]] = llvm.trunc %[[VAL_40]] : i32 to i8
168169
// CHECK: %[[VAL_42:.*]] = llvm.insertvalue %[[VAL_41]], %[[VAL_39]][6] : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>, ptr, array<1 x i64>)>
169-
// CHECK: %[[VAL_44:.*]] = llvm.insertvalue %[[VAL_27]], %[[VAL_42]][8] : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>, ptr, array<1 x i64>)>
170+
// CHECK: %[[VAL_44B:.*]] = llvm.insertvalue %[[VAL_27]], %[[VAL_42]][8] : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>, ptr, array<1 x i64>)>
171+
// CHECK: %[[VAL_44:.*]] = llvm.insertvalue %{{.*}}, %[[VAL_44B]][9, 0] : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>, ptr, array<1 x i64>)>
170172
// CHECK: %[[VAL_45:.*]] = llvm.getelementptr %[[VAL_3]][0, 7, 0, 0] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>, ptr, array<1 x i64>)>
171173
// CHECK: %[[VAL_46:.*]] = llvm.load %[[VAL_45]] {tbaa = [#[[$BOXT]]]} : !llvm.ptr -> i64
172174
// CHECK: %[[VAL_47:.*]] = llvm.getelementptr %[[VAL_3]][0, 7, 0, 1] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>, ptr, array<1 x i64>)>
@@ -218,7 +220,8 @@ module {
218220
// CHECK: %[[VAL_17:.*]] = llvm.trunc %[[VAL_16]] : i32 to i8
219221
// CHECK: %[[VAL_18:.*]] = llvm.insertvalue %[[VAL_17]], %[[VAL_15]][6] : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>, ptr, array<1 x i64>)>
220222
// CHECK: %[[VAL_19:.*]] = llvm.mlir.zero : !llvm.ptr
221-
// CHECK: %[[VAL_21:.*]] = llvm.insertvalue %[[VAL_19]], %[[VAL_18]][8] : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>, ptr, array<1 x i64>)>
223+
// CHECK: %[[VAL_21B:.*]] = llvm.insertvalue %[[VAL_19]], %[[VAL_18]][8] : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>, ptr, array<1 x i64>)>
224+
// CHECK: %[[VAL_21:.*]] = llvm.insertvalue %{{.*}}, %[[VAL_21B]][9, 0] : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>, ptr, array<1 x i64>)>
222225
// CHECK: %[[VAL_22:.*]] = llvm.mlir.constant(0 : i64) : i64
223226
// CHECK: %[[VAL_23:.*]] = llvm.mlir.constant(1 : i64) : i64
224227
// CHECK: %[[VAL_24:.*]] = llvm.insertvalue %[[VAL_23]], %[[VAL_21]][7, 0, 0] : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>, ptr, array<1 x i64>)>

‎flang/test/Fir/type-descriptor.fir

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ fir.global internal @_QFfooEx : !fir.box<!fir.heap<!sometype>> {
1414
}
1515
// CHECK: @_QFfooEx = internal global { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }
1616
// CHECK-SAME: { ptr null, i64 ptrtoint (ptr getelementptr (%_QFfooTsometype, ptr null, i32 1) to i64),
17-
// CHECK-SAME: i32 20180515, i8 0, i8 42, i8 2, i8 1, ptr @_QFfooE.dt.sometype, [1 x i64] undef }
17+
// CHECK-SAME: i32 20180515, i8 0, i8 42, i8 2, i8 1, ptr @_QFfooE.dt.sometype, [1 x i64] zeroinitializer }
1818

1919
!some_pdt_type = !fir.type<_QFfooTsome_pdt_typeK42K43{num:i32,values:!fir.box<!fir.ptr<!fir.array<?x?xf32>>>}>
2020
fir.global internal @_QFfooE.dt.some_pdt_type.42.43 constant : i8
@@ -26,4 +26,4 @@ fir.global internal @_QFfooEx2 : !fir.box<!fir.heap<!some_pdt_type>> {
2626
}
2727
// CHECK: @_QFfooEx2 = internal global { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }
2828
// CHECK-SAME: { ptr null, i64 ptrtoint (ptr getelementptr (%_QFfooTsome_pdt_typeK42K43, ptr null, i32 1) to i64),
29-
// CHECK-SAME: i32 20180515, i8 0, i8 42, i8 2, i8 1, ptr @_QFfooE.dt.some_pdt_type.42.43, [1 x i64] undef }
29+
// CHECK-SAME: i32 20180515, i8 0, i8 42, i8 2, i8 1, ptr @_QFfooE.dt.some_pdt_type.42.43, [1 x i64] zeroinitializer }

‎flang/test/Lower/allocatable-polymorphic.f90

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,8 @@ program test_alloc
649649
! LLVM: %[[BOX3:.*]] = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX2]], i8 %[[TYPE_CODE_TRUNC]], 4
650650
! LLVM: %[[BOX4:.*]] = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX3]], i8 0, 5
651651
! LLVM: %[[BOX5:.*]] = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX4]], i8 1, 6
652-
! LLVM: %[[BOX6:.*]] = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX5]], ptr %[[TDESC_C3]], 7
652+
! LLVM: %[[BOX6A:.*]] = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX5]], ptr %[[TDESC_C3]], 7
653+
! LLVM: %[[BOX6:.*]] = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX6A]], i64 0, 8, 0
653654
! LLVM: %[[BOX7:.*]] = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX6]], ptr %{{.*}}, 0
654655
! LLVM: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX7]], ptr %{{.*}}
655656
! LLVM: call void %{{.*}}(ptr %{{.*}})
@@ -669,7 +670,8 @@ program test_alloc
669670
! LLVM: %[[BOX3:.*]] = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX2]], i8 %[[TYPE_CODE_TRUNC]], 4
670671
! LLVM: %[[BOX4:.*]] = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX3]], i8 0, 5
671672
! LLVM: %[[BOX5:.*]] = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX4]], i8 1, 6
672-
! LLVM: %[[BOX6:.*]] = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX5]], ptr %[[TDESC_C4]], 7
673+
! LLVM: %[[BOX6A:.*]] = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX5]], ptr %[[TDESC_C4]], 7
674+
! LLVM: %[[BOX6:.*]] = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX6A]], i64 0, 8, 0
673675
! LLVM: %[[BOX7:.*]] = insertvalue { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX6]], ptr %{{.*}}, 0
674676
! LLVM: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[BOX7]], ptr %{{.*}}
675677
! LLVM: call void %{{.*}}(ptr %{{.*}})
@@ -679,7 +681,7 @@ program test_alloc
679681
! allocatable.
680682

681683
! LLVM-LABEL: define void @_QMpolyPtest_deallocate()
682-
! LLVM: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } { ptr null, i64 ptrtoint (ptr getelementptr (%_QMpolyTp1, ptr null, i32 1) to i64), i32 20180515, i8 0, i8 42, i8 2, i8 1, ptr @_QMpolyE.dt.p1, [1 x i64] undef }, ptr %[[ALLOCA1:[0-9]*]]
684+
! LLVM: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } { ptr null, i64 ptrtoint (ptr getelementptr (%_QMpolyTp1, ptr null, i32 1) to i64), i32 20180515, i8 0, i8 42, i8 2, i8 1, ptr @_QMpolyE.dt.p1, [1 x i64] zeroinitializer }, ptr %[[ALLOCA1:[0-9]*]]
683685
! LLVM: %[[LOAD:.*]] = load { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }, ptr %[[ALLOCA1]]
684686
! LLVM: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[LOAD]], ptr %[[ALLOCA2:[0-9]*]]
685687
! LLVM: %{{.*}} = call {} @_FortranAAllocatableInitDerivedForAllocate(ptr %[[ALLOCA2]], ptr @_QMpolyE.dt.p1, i32 0, i32 0)

0 commit comments

Comments
 (0)