[flang-commits] [flang] 15da136 - [flang] Add TODO for creation of polymorphic temporary

Jean Perier via flang-commits flang-commits at lists.llvm.org
Mon Apr 3 00:22:54 PDT 2023


Author: Jean Perier
Date: 2023-04-03T09:21:45+02:00
New Revision: 15da136f091939357f0178f360da74547aa6145a

URL: https://github.com/llvm/llvm-project/commit/15da136f091939357f0178f360da74547aa6145a
DIFF: https://github.com/llvm/llvm-project/commit/15da136f091939357f0178f360da74547aa6145a.diff

LOG: [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

Added: 
    

Modified: 
    flang/lib/Lower/ConvertExpr.cpp
    flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp
    flang/test/HLFIR/as_expr-codegen.fir
    flang/test/Lower/polymorphic.f90

Removed: 
    


################################################################################
diff  --git a/flang/lib/Lower/ConvertExpr.cpp b/flang/lib/Lower/ConvertExpr.cpp
index 9563911853a67..9bb158de7d7d3 100644
--- a/flang/lib/Lower/ConvertExpr.cpp
+++ b/flang/lib/Lower/ConvertExpr.cpp
@@ -2058,17 +2058,7 @@ class ScalarExprLowering {
           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");

diff  --git a/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp
index 44a61f0d9ef7d..1f096efdeff41 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<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"};

diff  --git a/flang/test/HLFIR/as_expr-codegen.fir b/flang/test/HLFIR/as_expr-codegen.fir
index c37c62469cefb..8c8d83b69426a 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<!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>

diff  --git a/flang/test/Lower/polymorphic.f90 b/flang/test/Lower/polymorphic.f90
index ff9dacc7b7d03..e8a120d7710f2 100644
--- a/flang/test/Lower/polymorphic.f90
+++ b/flang/test/Lower/polymorphic.f90
@@ -260,27 +260,12 @@ 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
 
@@ -1150,23 +1135,11 @@ 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
 


        


More information about the flang-commits mailing list