[flang] Add TODO for creation of polymorphic temporary
authorJean Perier <jperier@nvidia.com>
Mon, 3 Apr 2023 07:21:01 +0000 (09:21 +0200)
committerJean Perier <jperier@nvidia.com>
Mon, 3 Apr 2023 07:21:45 +0000 (09:21 +0200)
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
flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp
flang/test/HLFIR/as_expr-codegen.fir
flang/test/Lower/polymorphic.f90

index 9563911..9bb158d 100644 (file)
@@ -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<fir::LoadOp>(loc, value);
-          mlir::Value temp = builder.createTemporary(loc, value.getType());
-          builder.create<fir::StoreOp>(loc, value, temp);
-          mlir::Value empty;
-          mlir::ValueRange emptyRange;
-          auto boxTy = fir::ClassType::get(value.getType());
-          return builder.create<fir::EmboxOp>(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");
index 44a61f0..1f096ef 100644 (file)
@@ -98,6 +98,8 @@ static mlir::Value getBufferizedExprMustFreeFlag(mlir::Value bufferizedExpr) {
 static std::pair<hlfir::Entity, mlir::Value>
 createTempFromMold(mlir::Location loc, fir::FirOpBuilder &builder,
                    hlfir::Entity mold) {
+  if (mold.isPolymorphic())
+    TODO(loc, "creating polymorphic temporary");
   llvm::SmallVector<mlir::Value> lenParams;
   hlfir::genLengthParameters(loc, builder, mold, lenParams);
   llvm::StringRef tmpName{".tmp"};
index c37c624..8c8d83b 100644 (file)
@@ -54,20 +54,20 @@ func.func @shape_from_type(%arg0 : !fir.ref<!fir.array<10x20xi32>>) {
 // CHECK:    %[[VAL_9:.*]] = fir.insert_value %[[VAL_8]], %[[VAL_6]]#0, [0 : index] : (tuple<!fir.heap<!fir.array<10x20xi32>>, i1>, !fir.heap<!fir.array<10x20xi32>>) -> tuple<!fir.heap<!fir.array<10x20xi32>>, i1>
 
 
-func.func @shape_from_box(%arg0 : !fir.class<!fir.array<10x?xi32>>) {
-  %expr = hlfir.as_expr %arg0 : (!fir.class<!fir.array<10x?xi32>>) -> !hlfir.expr<10x?xi32>
+func.func @shape_from_box(%arg0 : !fir.box<!fir.array<10x?xi32>>) {
+  %expr = hlfir.as_expr %arg0 : (!fir.box<!fir.array<10x?xi32>>) -> !hlfir.expr<10x?xi32>
   return
 }
 // CHECK-LABEL:   func.func @shape_from_box(
-// CHECK-SAME:    %[[VAL_0:.*]]: !fir.class<!fir.array<10x?xi32>>) {
+// CHECK-SAME:    %[[VAL_0:.*]]: !fir.box<!fir.array<10x?xi32>>) {
 // 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<!fir.array<10x?xi32>>, index) -> (index, index, index)
+// CHECK:    %[[VAL_3:.*]]:3 = fir.box_dims %[[VAL_0]], %[[VAL_2]] : (!fir.box<!fir.array<10x?xi32>>, 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.array<10x?xi32>>, !fir.shape<2>) -> (!fir.box<!fir.array<10x?xi32>>, !fir.heap<!fir.array<10x?xi32>>)
-// CHECK:    hlfir.assign %[[VAL_0]] to %[[VAL_7]]#0 : !fir.class<!fir.array<10x?xi32>>, !fir.box<!fir.array<10x?xi32>>
+// CHECK:    hlfir.assign %[[VAL_0]] to %[[VAL_7]]#0 : !fir.box<!fir.array<10x?xi32>>, !fir.box<!fir.array<10x?xi32>>
 // CHECK:    %[[VAL_8:.*]] = fir.undefined tuple<!fir.box<!fir.array<10x?xi32>>, i1>
 // CHECK:    %[[VAL_9:.*]] = fir.insert_value %[[VAL_8]], %[[VAL_6]], [1 : index] : (tuple<!fir.box<!fir.array<10x?xi32>>, i1>, i1) -> tuple<!fir.box<!fir.array<10x?xi32>>, i1>
 // CHECK:    %[[VAL_10:.*]] = fir.insert_value %[[VAL_9]], %[[VAL_7]]#0, [0 : index] : (tuple<!fir.box<!fir.array<10x?xi32>>, i1>, !fir.box<!fir.array<10x?xi32>>) -> tuple<!fir.box<!fir.array<10x?xi32>>, i1>
index ff9dacc..e8a120d 100644 (file)
@@ -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.array<?x!fir.type<_QMpolymorphic_testTp1{a:i32,b:i32}>>> {fir.bindc_name = "a"}, %[[I:.*]]: !fir.ref<i32> {fir.bindc_name = "i"}) {
-! CHECK:  %[[TEMP:.*]] = fir.alloca !fir.type<_QMpolymorphic_testTp1{a:i32,b:i32}>
-! CHECK:  %[[LOADED_I:.*]] = fir.load %[[I]] : !fir.ref<i32>
-! 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<!fir.array<?x!fir.type<_QMpolymorphic_testTp1{a:i32,b:i32}>>>, i64) -> !fir.ref<!fir.type<_QMpolymorphic_testTp1{a:i32,b:i32}>>
-! CHECK:  %[[NO_REASSOC:.*]] = fir.no_reassoc %[[COORD]] : !fir.ref<!fir.type<_QMpolymorphic_testTp1{a:i32,b:i32}>>
-! CHECK:  %[[LOAD:.*]] = fir.load %[[NO_REASSOC]] : !fir.ref<!fir.type<_QMpolymorphic_testTp1{a:i32,b:i32}>>
-! CHECK:  fir.store %[[LOAD]] to %[[TEMP]] : !fir.ref<!fir.type<_QMpolymorphic_testTp1{a:i32,b:i32}>>
-! CHECK:  %[[EMBOX:.*]] = fir.embox %[[TEMP]] source_box %[[ARG0]] : (!fir.ref<!fir.type<_QMpolymorphic_testTp1{a:i32,b:i32}>>, !fir.class<!fir.array<?x!fir.type<_QMpolymorphic_testTp1{a:i32,b:i32}>>>) -> !fir.class<!fir.type<_QMpolymorphic_testTp1{a:i32,b:i32}>>
-! CHECK:  fir.call @_QMpolymorphic_testPtakes_p1(%[[EMBOX]]) {{.*}} : (!fir.class<!fir.type<_QMpolymorphic_testTp1{a:i32,b:i32}>>) -> ()
+! 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.type<_QMpolymorphic_testTp5{up:!fir.class<!fir.heap<none>>}>> {fir.bindc_name = "a"}) {
-! CHECK: %[[ALLOCA:.*]] = fir.alloca
-! CHECK: %[[FIELD_UP:.*]] = fir.field_index up, !fir.type<_QMpolymorphic_testTp5{up:!fir.class<!fir.heap<none>>}>
-! CHECK: %[[COORD:.*]] = fir.coordinate_of %[[ARG0]], %[[FIELD_UP]] : (!fir.ref<!fir.type<_QMpolymorphic_testTp5{up:!fir.class<!fir.heap<none>>}>>, !fir.field) -> !fir.ref<!fir.class<!fir.heap<none>>>
-! CHECK: %[[LOAD:.*]] = fir.load %[[COORD]] : !fir.ref<!fir.class<!fir.heap<none>>>
-! CHECK: %[[BOX_ADDR:.*]] = fir.box_addr %[[LOAD]] : (!fir.class<!fir.heap<none>>) -> !fir.heap<none>
-! CHECK: %[[LOAD_ADDR:.*]] = fir.load %[[BOX_ADDR]] : !fir.heap<none>
-! CHECK: %[[NO_REASSOC:.*]] = fir.no_reassoc %[[LOAD_ADDR]] : none
-! CHECK: fir.store %[[NO_REASSOC]] to %[[ALLOCA]] : !fir.ref<none>
-! CHECK: %[[EMBOX:.*]] = fir.embox %[[ALLOCA]] source_box %[[LOAD]] : (!fir.ref<none>, !fir.class<!fir.heap<none>>) -> !fir.class<none>
-! CHECK: fir.call @_QMpolymorphic_testPpass_up(%[[EMBOX]]) fastmath<contract> : (!fir.class<none>) -> ()
+! TODO: unlimited polymorphic temporary in lowering
+!  subroutine parenthesized_up(a)
+!    type(p5) :: a
+!    call pass_up((a%up))
+!  end subroutine
 
 end module