Skip to content

Commit 59e4d0b

Browse files
jeanPerierjeanPerier
authored
[flang][hlfir] ensure hlfir.declare result box attributes are consistent (#143137)
Prevent hlfir.declare output to be fir.box/class values with the heap/pointer attribute to ensure the runtime descriptor attributes are in line with the Fortran attributes for the entities being declared (only fir.ref<box/class> can be ALLOCATABLE/POINTERS). This fixes a bug where an associated entity inside a SELECT TYPE was being unexpectedly reallocated inside assign runtime because the selector was allocatable and this attribute was not properly removed when creating the descriptor for the associated entity (that does not inherit the ALLOCATABLE/POINTER attribute according to Fortran 2023 section 11.1.3.3).
1 parent 6582d7d commit 59e4d0b

17 files changed

+167
-117
lines changed

‎flang/include/flang/Optimizer/Dialect/FIRType.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ class BaseBoxType : public mlir::Type {
5959
/// Is this a box for a pointer?
6060
bool isPointer() const;
6161

62+
/// Does this box for a pointer or allocatable?
63+
bool isPointerOrAllocatable() const;
64+
6265
/// Is this a box describing volatile memory?
6366
bool isVolatile() const;
6467

‎flang/lib/Optimizer/Dialect/FIRType.cpp‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1482,10 +1482,12 @@ fir::BaseBoxType fir::BaseBoxType::getBoxTypeWithNewAttr(
14821482
break;
14831483
}
14841484
return llvm::TypeSwitch<fir::BaseBoxType, fir::BaseBoxType>(*this)
1485-
.Case<fir::BoxType>(
1486-
[baseType](auto) { return fir::BoxType::get(baseType); })
1487-
.Case<fir::ClassType>(
1488-
[baseType](auto) { return fir::ClassType::get(baseType); });
1485+
.Case<fir::BoxType>([baseType](auto b) {
1486+
return fir::BoxType::get(baseType, b.isVolatile());
1487+
})
1488+
.Case<fir::ClassType>([baseType](auto b) {
1489+
return fir::ClassType::get(baseType, b.isVolatile());
1490+
});
14891491
}
14901492

14911493
bool fir::BaseBoxType::isAssumedRank() const {
@@ -1499,7 +1501,12 @@ bool fir::BaseBoxType::isPointer() const {
14991501
return llvm::isa<fir::PointerType>(getEleTy());
15001502
}
15011503

1504+
bool fir::BaseBoxType::isPointerOrAllocatable() const {
1505+
return llvm::isa<fir::PointerType, fir::HeapType>(getEleTy());
1506+
}
1507+
15021508
bool BaseBoxType::isVolatile() const { return fir::isa_volatile_type(*this); }
1509+
15031510
//===----------------------------------------------------------------------===//
15041511
// FIROpsDialect
15051512
//===----------------------------------------------------------------------===//

‎flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp‎

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -180,26 +180,46 @@ void hlfir::AssignOp::getEffects(
180180
// DeclareOp
181181
//===----------------------------------------------------------------------===//
182182

183-
/// Given a FIR memory type, and information about non default lower bounds, get
184-
/// the related HLFIR variable type.
185-
mlir::Type hlfir::DeclareOp::getHLFIRVariableType(mlir::Type inputType,
186-
bool hasExplicitLowerBounds) {
183+
static std::pair<mlir::Type, mlir::Type>
184+
getDeclareOutputTypes(mlir::Type inputType, bool hasExplicitLowerBounds) {
185+
// Drop pointer/allocatable attribute of descriptor values. Only descriptor
186+
// addresses are ALLOCATABLE/POINTER. The HLFIR box result of an hlfir.declare
187+
// without those attributes should not have these attributes set.
188+
if (auto baseBoxType = mlir::dyn_cast<fir::BaseBoxType>(inputType))
189+
if (baseBoxType.isPointerOrAllocatable()) {
190+
mlir::Type boxWithoutAttributes =
191+
baseBoxType.getBoxTypeWithNewAttr(fir::BaseBoxType::Attribute::None);
192+
return {boxWithoutAttributes, boxWithoutAttributes};
193+
}
187194
mlir::Type type = fir::unwrapRefType(inputType);
188195
if (mlir::isa<fir::BaseBoxType>(type))
189-
return inputType;
196+
return {inputType, inputType};
190197
if (auto charType = mlir::dyn_cast<fir::CharacterType>(type))
191-
if (charType.hasDynamicLen())
192-
return fir::BoxCharType::get(charType.getContext(), charType.getFKind());
198+
if (charType.hasDynamicLen()) {
199+
mlir::Type hlfirType =
200+
fir::BoxCharType::get(charType.getContext(), charType.getFKind());
201+
return {hlfirType, inputType};
202+
}
193203

194204
auto seqType = mlir::dyn_cast<fir::SequenceType>(type);
195205
bool hasDynamicExtents =
196206
seqType && fir::sequenceWithNonConstantShape(seqType);
197207
mlir::Type eleType = seqType ? seqType.getEleTy() : type;
198208
bool hasDynamicLengthParams = fir::characterWithDynamicLen(eleType) ||
199209
fir::isRecordWithTypeParameters(eleType);
200-
if (hasExplicitLowerBounds || hasDynamicExtents || hasDynamicLengthParams)
201-
return fir::BoxType::get(type, fir::isa_volatile_type(inputType));
202-
return inputType;
210+
if (hasExplicitLowerBounds || hasDynamicExtents || hasDynamicLengthParams) {
211+
mlir::Type boxType =
212+
fir::BoxType::get(type, fir::isa_volatile_type(inputType));
213+
return {boxType, inputType};
214+
}
215+
return {inputType, inputType};
216+
}
217+
218+
/// Given a FIR memory type, and information about non default lower bounds, get
219+
/// the related HLFIR variable type.
220+
mlir::Type hlfir::DeclareOp::getHLFIRVariableType(mlir::Type inputType,
221+
bool hasExplicitLowerBounds) {
222+
return getDeclareOutputTypes(inputType, hasExplicitLowerBounds).first;
203223
}
204224

205225
static bool hasExplicitLowerBounds(mlir::Value shape) {
@@ -256,17 +276,18 @@ void hlfir::DeclareOp::build(mlir::OpBuilder &builder,
256276
std::tie(inputType, memref) = updateDeclaredInputTypeWithVolatility(
257277
inputType, memref, builder, flags);
258278
}
259-
mlir::Type hlfirVariableType =
260-
getHLFIRVariableType(inputType, hasExplicitLbs);
261-
build(builder, result, {hlfirVariableType, inputType}, memref, shape,
279+
auto [hlfirVariableType, firVarType] =
280+
getDeclareOutputTypes(inputType, hasExplicitLbs);
281+
build(builder, result, {hlfirVariableType, firVarType}, memref, shape,
262282
typeparams, dummy_scope, nameAttr, fortran_attrs, data_attr);
263283
}
264284

265285
llvm::LogicalResult hlfir::DeclareOp::verify() {
266-
if (getMemref().getType() != getResult(1).getType())
267-
return emitOpError("second result type must match input memref type");
268-
mlir::Type hlfirVariableType = getHLFIRVariableType(
286+
auto [hlfirVariableType, firVarType] = getDeclareOutputTypes(
269287
getMemref().getType(), hasExplicitLowerBounds(getShape()));
288+
if (firVarType != getResult(1).getType())
289+
return emitOpError("second result type must match input memref type, "
290+
"unless it is a box with heap or pointer attribute");
270291
if (hlfirVariableType != getResult(0).getType())
271292
return emitOpError("first result type is inconsistent with variable "
272293
"properties: expected ")
@@ -1608,7 +1629,7 @@ void hlfir::AssociateOp::build(
16081629
mlir::Type firVarType;
16091630
auto sourceExprType = mlir::dyn_cast<hlfir::ExprType>(source.getType());
16101631
if (sourceExprType && sourceExprType.isPolymorphic())
1611-
firVarType = fir::ClassType::get(fir::HeapType::get(dataType));
1632+
firVarType = fir::ClassType::get(dataType);
16121633
else
16131634
firVarType = fir::ReferenceType::get(dataType);
16141635

‎flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ class DeclareOpConversion : public mlir::OpRewritePattern<hlfir::DeclareOp> {
356356
// is used for accessing the bounds etc. Using the HLFIR box,
357357
// that holds the same base_addr at this point, makes
358358
// the representation a little bit more clear.
359-
if (hlfirBase.getType() == firBase.getType())
359+
if (hlfirBase.getType() == declareOp.getOriginalBase().getType())
360360
firBase = hlfirBase;
361361
} else {
362362
// Need to conditionally rebox/embox the optional: the input fir.box

‎flang/test/HLFIR/as_expr-codegen-polymorphic.fir‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@ func.func @as_expr_class(%arg0 : !fir.class<!t>, %arg1: !fir.ref<!t>) {
99
return
1010
}
1111
// CHECK-LABEL: func.func @as_expr_class(
12-
// CHECK: %[[VAL_6:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = ".tmp"} : (!fir.class<!fir.heap<!fir.type<t{i:i32}>>>) -> (!fir.class<!fir.heap<!fir.type<t{i:i32}>>>, !fir.class<!fir.heap<!fir.type<t{i:i32}>>>)
12+
// CHECK: %[[VAL_6:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = ".tmp"} : (!fir.class<!fir.heap<!fir.type<t{i:i32}>>>) -> (!fir.class<!fir.type<t{i:i32}>>, !fir.class<!fir.type<t{i:i32}>>)
1313
// CHECK: %[[VAL_5:.*]] = arith.constant true
1414
// ... copy ...
15-
// CHECK: %[[VAL_12:.*]] = fir.undefined tuple<!fir.class<!fir.heap<!fir.type<t{i:i32}>>>, i1>
16-
// CHECK: %[[VAL_13:.*]] = fir.insert_value %[[VAL_12]], %[[VAL_5]], [1 : index] : (tuple<!fir.class<!fir.heap<!fir.type<t{i:i32}>>>, i1>, i1) -> tuple<!fir.class<!fir.heap<!fir.type<t{i:i32}>>>, i1>
17-
// CHECK: %[[VAL_14:.*]] = fir.insert_value %[[VAL_13]], %[[VAL_6]]#0, [0 : index] : (tuple<!fir.class<!fir.heap<!fir.type<t{i:i32}>>>, i1>, !fir.class<!fir.heap<!fir.type<t{i:i32}>>>) -> tuple<!fir.class<!fir.heap<!fir.type<t{i:i32}>>>, i1>
18-
// CHECK: hlfir.assign %[[VAL_6]]#0 to %{{.*}} : !fir.class<!fir.heap<!fir.type<t{i:i32}>>>, !fir.ref<!fir.type<t{i:i32}>>
15+
// CHECK: %[[VAL_12:.*]] = fir.undefined tuple<!fir.class<!fir.type<t{i:i32}>>, i1>
16+
// CHECK: %[[VAL_13:.*]] = fir.insert_value %[[VAL_12]], %[[VAL_5]], [1 : index] : (tuple<!fir.class<!fir.type<t{i:i32}>>, i1>, i1) -> tuple<!fir.class<!fir.type<t{i:i32}>>, i1>
17+
// CHECK: %[[VAL_14:.*]] = fir.insert_value %[[VAL_13]], %[[VAL_6]]#0, [0 : index] : (tuple<!fir.class<!fir.type<t{i:i32}>>, i1>, !fir.class<!fir.type<t{i:i32}>>) -> tuple<!fir.class<!fir.type<t{i:i32}>>, i1>
18+
// CHECK: hlfir.assign %[[VAL_6]]#0 to %{{.*}} : !fir.class<!fir.type<t{i:i32}>>, !fir.ref<!fir.type<t{i:i32}>>
1919

2020

2121
func.func @as_expr_class_2(%arg0 : !fir.class<!fir.array<?x!t>>) {
@@ -25,11 +25,11 @@ func.func @as_expr_class_2(%arg0 : !fir.class<!fir.array<?x!t>>) {
2525
return
2626
}
2727
// CHECK-LABEL: func.func @as_expr_class_2(
28-
// CHECK: %[[VAL_10:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = ".tmp"} : (!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>) -> (!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>, !fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>)
28+
// CHECK: %[[VAL_10:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = ".tmp"} : (!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>) -> (!fir.class<!fir.array<?x!fir.type<t{i:i32}>>>, !fir.class<!fir.array<?x!fir.type<t{i:i32}>>>)
2929
// CHECK: %[[VAL_9:.*]] = arith.constant true
3030
// ... copy ...
31-
// CHECK: %[[VAL_16:.*]] = fir.undefined tuple<!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>, i1>
32-
// CHECK: %[[VAL_17:.*]] = fir.insert_value %[[VAL_16]], %[[VAL_9]], [1 : index] : (tuple<!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>, i1>, i1) -> tuple<!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>, i1>
33-
// CHECK: %[[VAL_18:.*]] = fir.insert_value %[[VAL_17]], %[[VAL_10]]#0, [0 : index] : (tuple<!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>, i1>, !fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>) -> tuple<!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>, i1>
31+
// CHECK: %[[VAL_16:.*]] = fir.undefined tuple<!fir.class<!fir.array<?x!fir.type<t{i:i32}>>>, i1>
32+
// CHECK: %[[VAL_17:.*]] = fir.insert_value %[[VAL_16]], %[[VAL_9]], [1 : index] : (tuple<!fir.class<!fir.array<?x!fir.type<t{i:i32}>>>, i1>, i1) -> tuple<!fir.class<!fir.array<?x!fir.type<t{i:i32}>>>, i1>
33+
// CHECK: %[[VAL_18:.*]] = fir.insert_value %[[VAL_17]], %[[VAL_10]]#0, [0 : index] : (tuple<!fir.class<!fir.array<?x!fir.type<t{i:i32}>>>, i1>, !fir.class<!fir.array<?x!fir.type<t{i:i32}>>>) -> tuple<!fir.class<!fir.array<?x!fir.type<t{i:i32}>>>, i1>
3434
// CHECK: %[[VAL_19:.*]] = arith.constant 1 : index
35-
// CHECK: %[[VAL_20:.*]] = hlfir.designate %[[VAL_10]]#0 (%[[VAL_19]]) : (!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>, index) -> !fir.class<!fir.type<t{i:i32}>>
35+
// CHECK: %[[VAL_20:.*]] = hlfir.designate %[[VAL_10]]#0 (%[[VAL_19]]) : (!fir.class<!fir.array<?x!fir.type<t{i:i32}>>>, index) -> !fir.class<!fir.type<t{i:i32}>>

‎flang/test/HLFIR/associate-codegen.fir‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -174,22 +174,22 @@ func.func @test_result_convert(%x : !fir.heap<!fir.array<10xi32>>) {
174174

175175
func.func @test_0dim_box(%x : !fir.ref<!fir.box<!fir.heap<i32>>>) {
176176
%0 = fir.load %x : !fir.ref<!fir.box<!fir.heap<i32>>>
177-
%1:2 = hlfir.declare %0 {uniq_name = ".tmp.intrinsic_result"} : (!fir.box<!fir.heap<i32>>) -> (!fir.box<!fir.heap<i32>>, !fir.box<!fir.heap<i32>>)
177+
%1:2 = hlfir.declare %0 {uniq_name = ".tmp.intrinsic_result"} : (!fir.box<!fir.heap<i32>>) -> (!fir.box<i32>, !fir.box<i32>)
178178
%true = arith.constant true
179-
%2 = hlfir.as_expr %1#0 move %true : (!fir.box<!fir.heap<i32>>, i1) -> !hlfir.expr<i32>
179+
%2 = hlfir.as_expr %1#0 move %true : (!fir.box<i32>, i1) -> !hlfir.expr<i32>
180180
%3:3 = hlfir.associate %2 {adapt.valuebyref} : (!hlfir.expr<i32>) -> (!fir.ref<i32>, !fir.ref<i32>, i1)
181181
return
182182
}
183183
// CHECK-LABEL: func.func @test_0dim_box(
184184
// CHECK-SAME: %[[VAL_0:.*]]: !fir.ref<!fir.box<!fir.heap<i32>>>) {
185185
// CHECK: %[[VAL_1:.*]] = fir.load %[[VAL_0]] : !fir.ref<!fir.box<!fir.heap<i32>>>
186-
// CHECK: %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_1]] {uniq_name = ".tmp.intrinsic_result"} : (!fir.box<!fir.heap<i32>>) -> (!fir.box<!fir.heap<i32>>, !fir.box<!fir.heap<i32>>)
186+
// CHECK: %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_1]] {uniq_name = ".tmp.intrinsic_result"} : (!fir.box<!fir.heap<i32>>) -> (!fir.box<i32>, !fir.box<i32>)
187187
// CHECK: %[[VAL_3:.*]] = arith.constant true
188-
// CHECK: %[[VAL_4:.*]] = fir.undefined tuple<!fir.box<!fir.heap<i32>>, i1>
189-
// CHECK: %[[VAL_5:.*]] = fir.insert_value %[[VAL_4]], %[[VAL_3]], [1 : index] : (tuple<!fir.box<!fir.heap<i32>>, i1>, i1) -> tuple<!fir.box<!fir.heap<i32>>, i1>
190-
// CHECK: %[[VAL_6:.*]] = fir.insert_value %[[VAL_5]], %[[VAL_2]]#0, [0 : index] : (tuple<!fir.box<!fir.heap<i32>>, i1>, !fir.box<!fir.heap<i32>>) -> tuple<!fir.box<!fir.heap<i32>>, i1>
191-
// CHECK: %[[VAL_7:.*]] = fir.box_addr %[[VAL_2]]#0 : (!fir.box<!fir.heap<i32>>) -> !fir.ref<i32>
192-
// CHECK: %[[VAL_8:.*]] = fir.box_addr %[[VAL_2]]#1 : (!fir.box<!fir.heap<i32>>) -> !fir.ref<i32>
188+
// CHECK: %[[VAL_4:.*]] = fir.undefined tuple<!fir.box<i32>, i1>
189+
// CHECK: %[[VAL_5:.*]] = fir.insert_value %[[VAL_4]], %[[VAL_3]], [1 : index] : (tuple<!fir.box<i32>, i1>, i1) -> tuple<!fir.box<i32>, i1>
190+
// CHECK: %[[VAL_6:.*]] = fir.insert_value %[[VAL_5]], %[[VAL_2]]#0, [0 : index] : (tuple<!fir.box<i32>, i1>, !fir.box<i32>) -> tuple<!fir.box<i32>, i1>
191+
// CHECK: %[[VAL_7:.*]] = fir.box_addr %[[VAL_2]]#0 : (!fir.box<i32>) -> !fir.ref<i32>
192+
// CHECK: %[[VAL_8:.*]] = fir.box_addr %[[VAL_2]]#1 : (!fir.box<i32>) -> !fir.ref<i32>
193193
// CHECK: return
194194
// CHECK: }
195195

0 commit comments

Comments
 (0)