From 15da136f091939357f0178f360da74547aa6145a Mon Sep 17 00:00:00 2001 From: Jean Perier Date: Mon, 3 Apr 2023 09:21:01 +0200 Subject: [PATCH] [flang] Add TODO for creation of polymorphic temporary The current code is wrong: it is doing an alloca with the declared type instead of the dynamic type, leading to undefined behavior when the dynamic type and declared type differ and the temporary is later used. This also introduces some `fir.alloca none` for unlimited polymorphic that are not allocating the right thing at all. Add TODOs for now, the correct thing to do will probably be to use the runtime (like AssignTemporary), but since this happens in code doing "mold" temp allocation, I first need to check if there is a need for "mold" temporary creation not followed by an assign, or if this can be combined with the assign instead (for HLFIR, it is pretty easy combine this from as_expr codegen, not sure for the current lowering). Differential Revision: https://reviews.llvm.org/D147333 --- flang/lib/Lower/ConvertExpr.cpp | 12 +----- .../Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp | 2 + flang/test/HLFIR/as_expr-codegen.fir | 10 ++--- flang/test/Lower/polymorphic.f90 | 49 +++++----------------- 4 files changed, 19 insertions(+), 54 deletions(-) diff --git a/flang/lib/Lower/ConvertExpr.cpp b/flang/lib/Lower/ConvertExpr.cpp index 9563911..9bb158d 100644 --- a/flang/lib/Lower/ConvertExpr.cpp +++ b/flang/lib/Lower/ConvertExpr.cpp @@ -2058,17 +2058,7 @@ public: return temp; }, [&](const fir::PolymorphicValue &p) -> ExtValue { - mlir::Type type = p.getAddr().getType(); - mlir::Value value = p.getAddr(); - if (fir::isa_ref_type(type)) - value = builder.create(loc, value); - mlir::Value temp = builder.createTemporary(loc, value.getType()); - builder.create(loc, value, temp); - mlir::Value empty; - mlir::ValueRange emptyRange; - auto boxTy = fir::ClassType::get(value.getType()); - return builder.create(loc, boxTy, temp, empty, empty, - emptyRange, p.getSourceBox()); + TODO(loc, "creating polymorphic temporary"); }, [&](const auto &) -> ExtValue { fir::emitFatalError(loc, "expr is not a scalar value"); diff --git a/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp index 44a61f0..1f096ef 100644 --- a/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp +++ b/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp @@ -98,6 +98,8 @@ static mlir::Value getBufferizedExprMustFreeFlag(mlir::Value bufferizedExpr) { static std::pair createTempFromMold(mlir::Location loc, fir::FirOpBuilder &builder, hlfir::Entity mold) { + if (mold.isPolymorphic()) + TODO(loc, "creating polymorphic temporary"); llvm::SmallVector lenParams; hlfir::genLengthParameters(loc, builder, mold, lenParams); llvm::StringRef tmpName{".tmp"}; diff --git a/flang/test/HLFIR/as_expr-codegen.fir b/flang/test/HLFIR/as_expr-codegen.fir index c37c624..8c8d83b 100644 --- a/flang/test/HLFIR/as_expr-codegen.fir +++ b/flang/test/HLFIR/as_expr-codegen.fir @@ -54,20 +54,20 @@ func.func @shape_from_type(%arg0 : !fir.ref>) { // CHECK: %[[VAL_9:.*]] = fir.insert_value %[[VAL_8]], %[[VAL_6]]#0, [0 : index] : (tuple>, i1>, !fir.heap>) -> tuple>, i1> -func.func @shape_from_box(%arg0 : !fir.class>) { - %expr = hlfir.as_expr %arg0 : (!fir.class>) -> !hlfir.expr<10x?xi32> +func.func @shape_from_box(%arg0 : !fir.box>) { + %expr = hlfir.as_expr %arg0 : (!fir.box>) -> !hlfir.expr<10x?xi32> return } // CHECK-LABEL: func.func @shape_from_box( -// CHECK-SAME: %[[VAL_0:.*]]: !fir.class>) { +// CHECK-SAME: %[[VAL_0:.*]]: !fir.box>) { // CHECK: %[[VAL_1:.*]] = arith.constant 10 : index // CHECK: %[[VAL_2:.*]] = arith.constant 1 : index -// CHECK: %[[VAL_3:.*]]:3 = fir.box_dims %[[VAL_0]], %[[VAL_2]] : (!fir.class>, index) -> (index, index, index) +// CHECK: %[[VAL_3:.*]]:3 = fir.box_dims %[[VAL_0]], %[[VAL_2]] : (!fir.box>, index) -> (index, index, index) // CHECK: %[[VAL_4:.*]] = fir.shape %[[VAL_1]], %[[VAL_3]]#1 : (index, index) -> !fir.shape<2> // CHECK: %[[VAL_5:.*]] = fir.allocmem !fir.array<10x?xi32>, %[[VAL_3]]#1 {bindc_name = ".tmp", uniq_name = ""} // CHECK: %[[VAL_6:.*]] = arith.constant true // CHECK: %[[VAL_7:.*]]:2 = hlfir.declare %[[VAL_5]](%[[VAL_4]]) {uniq_name = ".tmp"} : (!fir.heap>, !fir.shape<2>) -> (!fir.box>, !fir.heap>) -// CHECK: hlfir.assign %[[VAL_0]] to %[[VAL_7]]#0 : !fir.class>, !fir.box> +// CHECK: hlfir.assign %[[VAL_0]] to %[[VAL_7]]#0 : !fir.box>, !fir.box> // CHECK: %[[VAL_8:.*]] = fir.undefined tuple>, i1> // CHECK: %[[VAL_9:.*]] = fir.insert_value %[[VAL_8]], %[[VAL_6]], [1 : index] : (tuple>, i1>, i1) -> tuple>, i1> // CHECK: %[[VAL_10:.*]] = fir.insert_value %[[VAL_9]], %[[VAL_7]]#0, [0 : index] : (tuple>, i1>, !fir.box>) -> tuple>, i1> diff --git a/flang/test/Lower/polymorphic.f90 b/flang/test/Lower/polymorphic.f90 index ff9dacc..e8a120d 100644 --- a/flang/test/Lower/polymorphic.f90 +++ b/flang/test/Lower/polymorphic.f90 @@ -260,27 +260,12 @@ module polymorphic_test class(p1), intent(in) :: p end subroutine -! CHECK-LABEL: func.func @_QMpolymorphic_testPtakes_p1 - - subroutine no_reassoc_poly_value(a, i) - class(p1), intent(in) :: a(:) - integer :: i - call takes_p1((a(i))) - end subroutine - -! CHECK-LABEL: func.func @_QMpolymorphic_testPno_reassoc_poly_value( -! CHECK-SAME: %[[ARG0:.*]]: !fir.class>> {fir.bindc_name = "a"}, %[[I:.*]]: !fir.ref {fir.bindc_name = "i"}) { -! CHECK: %[[TEMP:.*]] = fir.alloca !fir.type<_QMpolymorphic_testTp1{a:i32,b:i32}> -! CHECK: %[[LOADED_I:.*]] = fir.load %[[I]] : !fir.ref -! CHECK: %[[I_I64:.*]] = fir.convert %[[LOADED_I]] : (i32) -> i64 -! CHECK: %[[C1:.*]] = arith.constant 1 : i64 -! CHECK: %[[IDX:.*]] = arith.subi %[[I_I64]], %[[C1]] : i64 -! CHECK: %[[COORD:.*]] = fir.coordinate_of %[[ARG0]], %[[IDX]] : (!fir.class>>, i64) -> !fir.ref> -! CHECK: %[[NO_REASSOC:.*]] = fir.no_reassoc %[[COORD]] : !fir.ref> -! CHECK: %[[LOAD:.*]] = fir.load %[[NO_REASSOC]] : !fir.ref> -! CHECK: fir.store %[[LOAD]] to %[[TEMP]] : !fir.ref> -! CHECK: %[[EMBOX:.*]] = fir.embox %[[TEMP]] source_box %[[ARG0]] : (!fir.ref>, !fir.class>>) -> !fir.class> -! CHECK: fir.call @_QMpolymorphic_testPtakes_p1(%[[EMBOX]]) {{.*}} : (!fir.class>) -> () +! TODO: implement polymorphic temporary in lowering +! subroutine no_reassoc_poly_value(a, i) +! class(p1), intent(in) :: a(:) +! integer :: i +! call takes_p1((a(i))) +! end subroutine ! Test pointer assignment with non polymorphic lhs and polymorphic rhs @@ -1150,23 +1135,11 @@ module polymorphic_test class(*), intent(in) :: up end subroutine - subroutine parenthesized_up(a) - type(p5) :: a - call pass_up((a%up)) - end subroutine - -! CHECK-LABEL: func.func @_QMpolymorphic_testPparenthesized_up( -! CHECK-SAME: %[[ARG0:.*]]: !fir.ref>}>> {fir.bindc_name = "a"}) { -! CHECK: %[[ALLOCA:.*]] = fir.alloca -! CHECK: %[[FIELD_UP:.*]] = fir.field_index up, !fir.type<_QMpolymorphic_testTp5{up:!fir.class>}> -! CHECK: %[[COORD:.*]] = fir.coordinate_of %[[ARG0]], %[[FIELD_UP]] : (!fir.ref>}>>, !fir.field) -> !fir.ref>> -! CHECK: %[[LOAD:.*]] = fir.load %[[COORD]] : !fir.ref>> -! CHECK: %[[BOX_ADDR:.*]] = fir.box_addr %[[LOAD]] : (!fir.class>) -> !fir.heap -! CHECK: %[[LOAD_ADDR:.*]] = fir.load %[[BOX_ADDR]] : !fir.heap -! CHECK: %[[NO_REASSOC:.*]] = fir.no_reassoc %[[LOAD_ADDR]] : none -! CHECK: fir.store %[[NO_REASSOC]] to %[[ALLOCA]] : !fir.ref -! CHECK: %[[EMBOX:.*]] = fir.embox %[[ALLOCA]] source_box %[[LOAD]] : (!fir.ref, !fir.class>) -> !fir.class -! CHECK: fir.call @_QMpolymorphic_testPpass_up(%[[EMBOX]]) fastmath : (!fir.class) -> () +! TODO: unlimited polymorphic temporary in lowering +! subroutine parenthesized_up(a) +! type(p5) :: a +! call pass_up((a%up)) +! end subroutine end module -- 2.7.4