From 3a4e9f7ae50980ce8f103cbe86d24c574c8c6cac Mon Sep 17 00:00:00 2001 From: Slava Zakharin Date: Mon, 26 Jun 2023 14:32:41 -0700 Subject: [PATCH] [flang][hlfir] Do not dereference unallocated entities in structure constructor. Component-by-component assignment must be able to handle unallocated allocatable values in structure constructor. F2018 7.5.10 p. 7 states that the component must have unallocated status as a result of such construction. The structure constructor temporary is initialized such that all the allocatable components are unallocated, so we just need to make sure not to do the component assignment if RHS is deallocated. Depends on D152482 (the same LIT test is affected) Reviewed By: jeanPerier, tblah Differential Revision: https://reviews.llvm.org/D152493 --- flang/lib/Lower/ConvertExprToHLFIR.cpp | 38 ++++++++-- flang/runtime/assign.cpp | 13 +++- flang/test/Lower/HLFIR/structure-constructor.f90 | 90 ++++++++++++++---------- 3 files changed, 99 insertions(+), 42 deletions(-) diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp index a2774de..7df4a62 100644 --- a/flang/lib/Lower/ConvertExprToHLFIR.cpp +++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp @@ -1743,10 +1743,40 @@ private: attrs && bitEnumContainsAny(attrs.getFlags(), fir::FortranVariableFlagsEnum::allocatable); - auto rhs = gen(expr); - builder.create(loc, rhs, lhs, allowRealloc, - /*keep_lhs_length_if_realloc=*/false, - /*temporary_lhs=*/true); + hlfir::Entity rhs = gen(expr); + // If the component is allocatable, then we have to check + // whether the RHS value is allocatable or not. + // If it is not allocatable, then AssignOp can be used directly. + // If it is allocatable, then using AssignOp for unallocated RHS + // will cause illegal dereference. When an unallocated allocatable + // value is used to construct an allocatable component, the component + // must just stay unallocated. + if (!allowRealloc || !rhs.isMutableBox()) { + rhs = hlfir::loadTrivialScalar(loc, builder, rhs); + builder.create(loc, rhs, lhs, allowRealloc, + /*keep_lhs_length_if_realloc=*/false, + /*temporary_lhs=*/true); + continue; + } + + auto [rhsExv, cleanup] = + hlfir::translateToExtendedValue(loc, builder, rhs); + assert(!cleanup && "unexpected cleanup"); + auto *fromBox = rhsExv.getBoxOf(); + if (!fromBox) + fir::emitFatalError(loc, "allocatable entity could not be lowered " + "to mutable box"); + mlir::Value isAlloc = + fir::factory::genIsAllocatedOrAssociatedTest(builder, loc, *fromBox); + builder.genIfThen(loc, isAlloc) + .genThen([&]() { + rhs = hlfir::loadTrivialScalar(loc, builder, rhs); + builder.create( + loc, rhs, lhs, allowRealloc, + /*keep_lhs_length_if_realloc=*/false, + /*temporary_lhs=*/true); + }) + .end(); } return varOp; diff --git a/flang/runtime/assign.cpp b/flang/runtime/assign.cpp index 80ad97c..8c1ee29 100644 --- a/flang/runtime/assign.cpp +++ b/flang/runtime/assign.cpp @@ -560,7 +560,18 @@ void RTNAME(AssignTemporary)(Descriptor &to, const Descriptor &from, // Initialize the "to" if it is of derived type that needs initialization. if (const DescriptorAddendum * addendum{to.Addendum()}) { if (const auto *derived{addendum->derivedType()}) { - if (!derived->noInitializationNeeded()) { + // Do not invoke the initialization, if the descriptor is unallocated. + // AssignTemporary() is used for component-by-component assignments, + // for example, for structure constructors. This means that the LHS + // may be an allocatable component with unallocated status. + // The initialization will just fail in this case. By skipping + // the initialization we let Assign() automatically allocate + // and initialize the component according to the RHS. + // So we only need to initialize the LHS here if it is allocated. + // Note that initializing already initialized entity has no visible + // effect, though, it is assumed that the compiler does not initialize + // the temporary and leaves the initialization to this runtime code. + if (!derived->noInitializationNeeded() && to.IsAllocated()) { if (ReturnError(terminator, Initialize(to, *derived, terminator)) != StatOk) { return; diff --git a/flang/test/Lower/HLFIR/structure-constructor.f90 b/flang/test/Lower/HLFIR/structure-constructor.f90 index 5eff0fc..53b7c9d 100644 --- a/flang/test/Lower/HLFIR/structure-constructor.f90 +++ b/flang/test/Lower/HLFIR/structure-constructor.f90 @@ -194,7 +194,15 @@ end subroutine test5 ! CHECK: %[[VAL_16:.*]] = fir.convert %[[VAL_13]] : (!fir.ref>) -> !fir.ref ! CHECK: %[[VAL_17:.*]] = fir.call @_FortranAInitialize(%[[VAL_15]], %[[VAL_16]], %[[VAL_14]]) fastmath : (!fir.box, !fir.ref, i32) -> none ! CHECK: %[[VAL_18:.*]] = hlfir.designate %[[VAL_11]]#0{"t5m"} {fortran_attrs = #fir.var_attrs} : (!fir.ref>>>}>>>>}>>) -> !fir.ref>>>}>>>>> -! CHECK: hlfir.assign %[[VAL_10]]#0 to %[[VAL_18]] realloc temporary_lhs : !fir.ref>>>}>>>>>, !fir.ref>>>}>>>>> +! CHECK: %[[VAL_19:.*]] = fir.load %[[VAL_10]]#1 : !fir.ref>>>}>>>>> +! CHECK: %[[VAL_20:.*]] = fir.box_addr %[[VAL_19]] : (!fir.box>>>}>>>>) -> !fir.heap>>>}>>> +! CHECK: %[[VAL_21:.*]] = fir.convert %[[VAL_20]] : (!fir.heap>>>}>>>) -> i64 +! CHECK: %[[VAL_22:.*]] = arith.constant 0 : i64 +! CHECK: %[[VAL_23:.*]] = arith.cmpi ne, %[[VAL_21]], %[[VAL_22]] : i64 +! CHECK: fir.if %[[VAL_23]] { +! CHECK: %[[VAL_24:.*]] = fir.load %[[VAL_10]]#0 : !fir.ref>>>}>>>>> +! CHECK: hlfir.assign %[[VAL_24]] to %[[VAL_18]] realloc temporary_lhs : !fir.box>>>}>>>>, !fir.ref>>>}>>>>> +! CHECK: } ! CHECK: hlfir.assign %[[VAL_11]]#0 to %[[VAL_3]]#0 : !fir.ref>>>}>>>>}>>, !fir.ref>>>}>>>>}>> ! CHECK: return ! CHECK: } @@ -243,43 +251,51 @@ end subroutine test6 ! CHECK: %[[VAL_33:.*]] = fir.convert %[[VAL_30]] : (!fir.ref>) -> !fir.ref ! CHECK: %[[VAL_34:.*]] = fir.call @_FortranAInitialize(%[[VAL_32]], %[[VAL_33]], %[[VAL_31]]) fastmath : (!fir.box, !fir.ref, i32) -> none ! CHECK: %[[VAL_35:.*]] = hlfir.designate %[[VAL_28]]#0{"t5m"} {fortran_attrs = #fir.var_attrs} : (!fir.ref>>>}>>>>}>>) -> !fir.ref>>>}>>>>> -! CHECK: hlfir.assign %[[VAL_19]]#0 to %[[VAL_35]] realloc temporary_lhs : !fir.ref>>>}>>>>>, !fir.ref>>>}>>>>> +! CHECK: %[[VAL_36:.*]] = fir.load %[[VAL_19]]#1 : !fir.ref>>>}>>>>> +! CHECK: %[[VAL_37:.*]] = fir.box_addr %[[VAL_36]] : (!fir.box>>>}>>>>) -> !fir.heap>>>}>>> +! CHECK: %[[VAL_38:.*]] = fir.convert %[[VAL_37]] : (!fir.heap>>>}>>>) -> i64 +! CHECK: %[[VAL_39:.*]] = arith.constant 0 : i64 +! CHECK: %[[VAL_40:.*]] = arith.cmpi ne, %[[VAL_38]], %[[VAL_39]] : i64 +! CHECK: fir.if %[[VAL_40]] { +! CHECK: %[[VAL_41:.*]] = fir.load %[[VAL_19]]#0 : !fir.ref>>>}>>>>> +! CHECK: hlfir.assign %[[VAL_41]] to %[[VAL_35]] realloc temporary_lhs : !fir.box>>>}>>>>, !fir.ref>>>}>>>>> +! CHECK: } ! CHECK: hlfir.assign %[[VAL_28]]#0 to %[[VAL_27]] temporary_lhs : !fir.ref>>>}>>>>}>>, !fir.ref>>>}>>>>}>> -! CHECK: %[[VAL_36:.*]] = arith.constant 1 : index -! CHECK: %[[VAL_37:.*]] = fir.shape %[[VAL_36]] : (index) -> !fir.shape<1> -! CHECK: %[[VAL_38:.*]] = hlfir.designate %[[VAL_20]]#0{"t6m"} <%[[VAL_37]]> shape %[[VAL_37]] : (!fir.ref>>>}>>>>,t6m:!fir.array<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>}>>, !fir.shape<1>, !fir.shape<1>) -> !fir.ref}>>> -! CHECK: %[[VAL_39:.*]] = arith.constant 1 : index -! CHECK: %[[VAL_40:.*]] = fir.allocmem !fir.array<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>> {bindc_name = ".tmp.arrayctor", uniq_name = ""} -! CHECK: %[[VAL_41:.*]] = fir.shape %[[VAL_39]] : (index) -> !fir.shape<1> -! CHECK: %[[VAL_42:.*]]:2 = hlfir.declare %[[VAL_40]](%[[VAL_41]]) {uniq_name = ".tmp.arrayctor"} : (!fir.heap}>>>, !fir.shape<1>) -> (!fir.heap}>>>, !fir.heap}>>>) -! CHECK: %[[VAL_43:.*]] = fir.embox %[[VAL_42]]#1(%[[VAL_41]]) : (!fir.heap}>>>, !fir.shape<1>) -> !fir.box}>>>> -! CHECK: fir.store %[[VAL_43]] to %[[VAL_4]] : !fir.ref}>>>>> -! CHECK: %[[VAL_44:.*]] = arith.constant false -! CHECK: %[[VAL_45:.*]] = fir.convert %[[VAL_3]] : (!fir.ref>) -> !fir.llvm_ptr -! CHECK: %[[VAL_46:.*]] = arith.constant 80 : i32 -! CHECK: %[[VAL_47:.*]] = fir.address_of(@_QQcl.{{.*}}) : !fir.ref> -! CHECK: %[[VAL_48:.*]] = arith.constant {{[0-9]*}} : i32 -! CHECK: %[[VAL_49:.*]] = fir.convert %[[VAL_4]] : (!fir.ref}>>>>>) -> !fir.ref> -! CHECK: %[[VAL_50:.*]] = fir.convert %[[VAL_47]] : (!fir.ref>) -> !fir.ref -! CHECK: %[[VAL_51:.*]] = fir.call @_FortranAInitArrayConstructorVector(%[[VAL_45]], %[[VAL_49]], %[[VAL_44]], %[[VAL_46]], %[[VAL_50]], %[[VAL_48]]) fastmath : (!fir.llvm_ptr, !fir.ref>, i1, i32, !fir.ref, i32) -> none -! CHECK: %[[VAL_52:.*]]:2 = hlfir.declare %[[VAL_2]] {uniq_name = "ctor.temp"} : (!fir.ref}>>) -> (!fir.ref}>>, !fir.ref}>>) -! CHECK: %[[VAL_53:.*]] = fir.embox %[[VAL_52]]#0 : (!fir.ref}>>) -> !fir.box}>> -! CHECK: %[[VAL_54:.*]] = fir.address_of(@_QQcl.{{.*}}) : !fir.ref> -! CHECK: %[[VAL_55:.*]] = arith.constant {{[0-9]*}} : i32 -! CHECK: %[[VAL_56:.*]] = fir.convert %[[VAL_53]] : (!fir.box}>>) -> !fir.box -! CHECK: %[[VAL_57:.*]] = fir.convert %[[VAL_54]] : (!fir.ref>) -> !fir.ref -! CHECK: %[[VAL_58:.*]] = fir.call @_FortranAInitialize(%[[VAL_56]], %[[VAL_57]], %[[VAL_55]]) fastmath : (!fir.box, !fir.ref, i32) -> none -! CHECK: %[[VAL_59:.*]] = arith.constant 4 : index -! CHECK: %[[VAL_60:.*]] = hlfir.designate %[[VAL_52]]#0{"c"} typeparams %[[VAL_59]] : (!fir.ref}>>, index) -> !fir.ref> -! CHECK: %[[VAL_61:.*]] = arith.constant 4 : i64 -! CHECK: %[[VAL_62:.*]] = hlfir.set_length %[[VAL_10]]#0 len %[[VAL_61]] : (!fir.ref>, i64) -> !hlfir.expr> -! CHECK: hlfir.assign %[[VAL_62]] to %[[VAL_60]] temporary_lhs : !hlfir.expr>, !fir.ref> -! CHECK: %[[VAL_63:.*]] = fir.convert %[[VAL_52]]#1 : (!fir.ref}>>) -> !fir.llvm_ptr -! CHECK: %[[VAL_64:.*]] = fir.call @_FortranAPushArrayConstructorSimpleScalar(%[[VAL_45]], %[[VAL_63]]) fastmath : (!fir.llvm_ptr, !fir.llvm_ptr) -> none -! CHECK: %[[VAL_65:.*]] = arith.constant true -! CHECK: %[[VAL_66:.*]] = hlfir.as_expr %[[VAL_42]]#0 move %[[VAL_65]] : (!fir.heap}>>>, i1) -> !hlfir.expr<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>> -! CHECK: hlfir.assign %[[VAL_66]] to %[[VAL_38]] temporary_lhs : !hlfir.expr<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>, !fir.ref}>>> +! CHECK: %[[VAL_42:.*]] = arith.constant 1 : index +! CHECK: %[[VAL_43:.*]] = fir.shape %[[VAL_42]] : (index) -> !fir.shape<1> +! CHECK: %[[VAL_44:.*]] = hlfir.designate %[[VAL_20]]#0{"t6m"} <%[[VAL_43]]> shape %[[VAL_43]] : (!fir.ref>>>}>>>>,t6m:!fir.array<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>}>>, !fir.shape<1>, !fir.shape<1>) -> !fir.ref}>>> +! CHECK: %[[VAL_45:.*]] = arith.constant 1 : index +! CHECK: %[[VAL_46:.*]] = fir.allocmem !fir.array<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>> {bindc_name = ".tmp.arrayctor", uniq_name = ""} +! CHECK: %[[VAL_47:.*]] = fir.shape %[[VAL_45]] : (index) -> !fir.shape<1> +! CHECK: %[[VAL_48:.*]]:2 = hlfir.declare %[[VAL_46]](%[[VAL_47]]) {uniq_name = ".tmp.arrayctor"} : (!fir.heap}>>>, !fir.shape<1>) -> (!fir.heap}>>>, !fir.heap}>>>) +! CHECK: %[[VAL_49:.*]] = fir.embox %[[VAL_48]]#1(%[[VAL_47]]) : (!fir.heap}>>>, !fir.shape<1>) -> !fir.box}>>>> +! CHECK: fir.store %[[VAL_49]] to %[[VAL_4]] : !fir.ref}>>>>> +! CHECK: %[[VAL_50:.*]] = arith.constant false +! CHECK: %[[VAL_51:.*]] = fir.convert %[[VAL_3]] : (!fir.ref>) -> !fir.llvm_ptr +! CHECK: %[[VAL_52:.*]] = arith.constant 80 : i32 +! CHECK: %[[VAL_53:.*]] = fir.address_of(@_QQcl.{{.*}}) : !fir.ref> +! CHECK: %[[VAL_54:.*]] = arith.constant {{[0-9]*}} : i32 +! CHECK: %[[VAL_55:.*]] = fir.convert %[[VAL_4]] : (!fir.ref}>>>>>) -> !fir.ref> +! CHECK: %[[VAL_56:.*]] = fir.convert %[[VAL_53]] : (!fir.ref>) -> !fir.ref +! CHECK: %[[VAL_57:.*]] = fir.call @_FortranAInitArrayConstructorVector(%[[VAL_51]], %[[VAL_55]], %[[VAL_50]], %[[VAL_52]], %[[VAL_56]], %[[VAL_54]]) fastmath : (!fir.llvm_ptr, !fir.ref>, i1, i32, !fir.ref, i32) -> none +! CHECK: %[[VAL_58:.*]]:2 = hlfir.declare %[[VAL_2]] {uniq_name = "ctor.temp"} : (!fir.ref}>>) -> (!fir.ref}>>, !fir.ref}>>) +! CHECK: %[[VAL_59:.*]] = fir.embox %[[VAL_58]]#0 : (!fir.ref}>>) -> !fir.box}>> +! CHECK: %[[VAL_60:.*]] = fir.address_of(@_QQcl.{{.*}}) : !fir.ref> +! CHECK: %[[VAL_61:.*]] = arith.constant {{[0-9]*}} : i32 +! CHECK: %[[VAL_62:.*]] = fir.convert %[[VAL_59]] : (!fir.box}>>) -> !fir.box +! CHECK: %[[VAL_63:.*]] = fir.convert %[[VAL_60]] : (!fir.ref>) -> !fir.ref +! CHECK: %[[VAL_64:.*]] = fir.call @_FortranAInitialize(%[[VAL_62]], %[[VAL_63]], %[[VAL_61]]) fastmath : (!fir.box, !fir.ref, i32) -> none +! CHECK: %[[VAL_65:.*]] = arith.constant 4 : index +! CHECK: %[[VAL_66:.*]] = hlfir.designate %[[VAL_58]]#0{"c"} typeparams %[[VAL_65]] : (!fir.ref}>>, index) -> !fir.ref> +! CHECK: %[[VAL_67:.*]] = arith.constant 4 : i64 +! CHECK: %[[VAL_68:.*]] = hlfir.set_length %[[VAL_10]]#0 len %[[VAL_67]] : (!fir.ref>, i64) -> !hlfir.expr> +! CHECK: hlfir.assign %[[VAL_68]] to %[[VAL_66]] temporary_lhs : !hlfir.expr>, !fir.ref> +! CHECK: %[[VAL_69:.*]] = fir.convert %[[VAL_58]]#1 : (!fir.ref}>>) -> !fir.llvm_ptr +! CHECK: %[[VAL_70:.*]] = fir.call @_FortranAPushArrayConstructorSimpleScalar(%[[VAL_51]], %[[VAL_69]]) fastmath : (!fir.llvm_ptr, !fir.llvm_ptr) -> none +! CHECK: %[[VAL_71:.*]] = arith.constant true +! CHECK: %[[VAL_72:.*]] = hlfir.as_expr %[[VAL_48]]#0 move %[[VAL_71]] : (!fir.heap}>>>, i1) -> !hlfir.expr<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>> +! CHECK: hlfir.assign %[[VAL_72]] to %[[VAL_44]] temporary_lhs : !hlfir.expr<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>, !fir.ref}>>> ! CHECK: hlfir.assign %[[VAL_20]]#0 to %[[VAL_12]]#0 : !fir.ref>>>}>>>>,t6m:!fir.array<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>}>>, !fir.ref>>>}>>>>,t6m:!fir.array<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>}>> -! CHECK: hlfir.destroy %[[VAL_66]] : !hlfir.expr<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>> +! CHECK: hlfir.destroy %[[VAL_72]] : !hlfir.expr<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>> ! CHECK: return ! CHECK: } -- 2.7.4