[flang] Add TODO for creation of polymorphic temporary

[jeanPerier]

Summary

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).

[clementval]

LGTM

[PeteSteinfeld]

All builds and tests correctly and looks good.

flang/lib/Lower/ConvertExpr.cpp

return genExtValue(expr).match(
           mlir::Value var =
               fir::getBase(fir::factory::readBoxValue(builder, loc, x));
           auto value = builder.create<fir::LoadOp>(loc, var);
           mlir::Value temp = builder.createTemporary(loc, value.getType());
           builder.create<fir::StoreOp>(loc, value, temp);
           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");
         });
   }
 
   /// Helper structure to track potential copy-in of non contiguous variable
   /// argument into a contiguous temp. It is used to deallocate the temp that

flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp

if (auto insert0 = insert.getAdt().getDefiningOp<fir::InsertValueOp>())
       if (insert0.getVal().getType() == tupleType.getType(1))
         return insert0.getVal();
   TODO(bufferizedExpr.getLoc(), "general extract storage case");
 }
 
 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"};
   mlir::Value alloc;
   mlir::Value isHeapAlloc;
   mlir::Value shape{};
   if (mold.isArray()) {
     mlir::Type sequenceType =

flang/test/HLFIR/as_expr-codegen.fir

 // CHECK:    %[[VAL_5:.*]] = arith.constant true
 // CHECK:    %[[VAL_6:.*]]:2 = hlfir.declare %[[VAL_4]](%[[VAL_3]]) {uniq_name = ".tmp"} : (!fir.heap<!fir.array<10x20xi32>>, !fir.shape<2>) -> (!fir.heap<!fir.array<10x20xi32>>, !fir.heap<!fir.array<10x20xi32>>)
 // CHECK:    hlfir.assign %[[VAL_0]] to %[[VAL_6]]#0 : !fir.ref<!fir.array<10x20xi32>>, !fir.heap<!fir.array<10x20xi32>>
 // CHECK:    %[[VAL_7:.*]] = fir.undefined tuple<!fir.heap<!fir.array<10x20xi32>>, i1>
 // CHECK:    %[[VAL_8:.*]] = fir.insert_value %[[VAL_7]], %[[VAL_5]], [1 : index] : (tuple<!fir.heap<!fir.array<10x20xi32>>, i1>, i1) -> tuple<!fir.heap<!fir.array<10x20xi32>>, i1>
 // 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>
 
 func.func @test_move(%arg0 : !fir.ref<!fir.array<10x20xi32>>, %must_free: i1) {
   %expr = hlfir.as_expr %arg0 move %must_free: (!fir.ref<!fir.array<10x20xi32>>, i1) -> !hlfir.expr<10x20xi32>
   return
 }
 // CHECK-LABEL:   func.func @test_move(
 // CHECK-SAME:    %[[VAL_0:.*]]: !fir.ref<!fir.array<10x20xi32>>,
 // CHECK-SAME:    %[[VAL_1:.*]]: i1) {
 // CHECK:    %[[VAL_2:.*]] = fir.undefined tuple<!fir.ref<!fir.array<10x20xi32>>, i1>
 // CHECK:    %[[VAL_3:.*]] = fir.insert_value %[[VAL_2]], %[[VAL_1]], [1 : index] : (tuple<!fir.ref<!fir.array<10x20xi32>>, i1>, i1) -> tuple<!fir.ref<!fir.array<10x20xi32>>, i1>
 // CHECK:    %[[VAL_4:.*]] = fir.insert_value %[[VAL_3]], %[[VAL_0]], [0 : index] : (tuple<!fir.ref<!fir.array<10x20xi32>>, i1>, !fir.ref<!fir.array<10x20xi32>>) -> tuple<!fir.ref<!fir.array<10x20xi32>>, i1>

flang/test/Lower/polymorphic.f90

subroutine derived_type_assignment_with_class()
     type(p3), target :: b(10)
     a = p3(b)
   end subroutine
 
   subroutine takes_p1(p)
     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
 
   subroutine pointer_assign_parent(p)
     type(p2), target :: p
     type(p1), pointer :: tp
     tp => p%p1
   end subroutine

 ! CHECK-LABEL: func.func @_QMpolymorphic_testPg(
 ! CHECK-SAME: %[[B:.*]]: !fir.class<!fir.array<?x!fir.type<_QMpolymorphic_testTp1{a:i32,b:i32}>>> {fir.bindc_name = "b"}) {
 ! CHECK: %[[A:.*]] = fir.alloca !fir.class<!fir.heap<!fir.array<?x!fir.type<_QMpolymorphic_testTp1{a:i32,b:i32}>>>>
 
   subroutine pass_up(up)
     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
 
 program test
   use polymorphic_test
   type(outer), allocatable :: o
   integer :: i(5)
   logical :: l(5)