[flang-commits] [flang] [flang] Fix hlfir.as_expr codegen for polymorphic entities (PR #80824)
via flang-commits
flang-commits at lists.llvm.org
Tue Feb 6 03:00:15 PST 2024
https://github.com/jeanPerier created https://github.com/llvm/llvm-project/pull/80824
https://github.com/llvm/llvm-project/pull/80683 revealed that hlfir.as_expr was propagating the temporary buffer for polymorphic values as an allocatable while codegen later expects to be working with fir.box/fir.class but not fir.ref<box/class> when processing the operations using the hlfir.as_expr result.
Dereference the temporary allocatable as soon as it is created.
>From 565129220f6088efd2b61b30520ab2fc19076441 Mon Sep 17 00:00:00 2001
From: Jean Perier <jperier at nvidia.com>
Date: Tue, 6 Feb 2024 02:51:34 -0800
Subject: [PATCH] [flang] Fix hlfir.as_expr codegen
https://github.com/llvm/llvm-project/pull/80683 revealed that
hlfir.as_expr was propagating the temporary buffer for polymorphic
values as an allocatable while further codegen on the result uses
expect to be working with fir.box/fir.class but not fir.ref<box/class>.
Dereference the temporary allocatable as soon as it is created.
---
.../HLFIR/Transforms/BufferizeHLFIR.cpp | 30 +++++++++------
.../HLFIR/as_expr-codegen-polymorphic.fir | 38 +++++++++++++++++++
flang/test/HLFIR/bufferize-poly-expr.fir | 25 ++++++------
3 files changed, 68 insertions(+), 25 deletions(-)
create mode 100644 flang/test/HLFIR/as_expr-codegen-polymorphic.fir
diff --git a/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp
index 15258e3e9c309..2eaed166190a5 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp
@@ -159,6 +159,22 @@ createArrayTemp(mlir::Location loc, fir::FirOpBuilder &builder,
return {hlfir::Entity{declareOp.getBase()}, trueVal};
}
+/// Copy \p source into a new temporary and package the temporary into a
+/// <temp,cleanup> tuple. The temporary may be heap of stack allocated.
+static mlir::Value copyInTempAndPackage(mlir::Location loc,
+ fir::FirOpBuilder &builder,
+ hlfir::Entity source) {
+ auto [temp, cleanup] = hlfir::createTempFromMold(loc, builder, source);
+ builder.create<hlfir::AssignOp>(loc, source, temp, temp.isAllocatable(),
+ /*keep_lhs_length_if_realloc=*/false,
+ /*temporary_lhs=*/true);
+ // Dereference allocatable temporary directly to simplify processing
+ // of its uses.
+ if (temp.isAllocatable())
+ temp = hlfir::derefPointersAndAllocatables(loc, builder, temp);
+ return packageBufferizedExpr(loc, builder, temp, cleanup);
+}
+
struct AsExprOpConversion : public mlir::OpConversionPattern<hlfir::AsExprOp> {
using mlir::OpConversionPattern<hlfir::AsExprOp>::OpConversionPattern;
explicit AsExprOpConversion(mlir::MLIRContext *ctx)
@@ -178,12 +194,7 @@ struct AsExprOpConversion : public mlir::OpConversionPattern<hlfir::AsExprOp> {
}
// Otherwise, create a copy in a new buffer.
hlfir::Entity source = hlfir::Entity{adaptor.getVar()};
- auto [temp, cleanup] = hlfir::createTempFromMold(loc, builder, source);
- builder.create<hlfir::AssignOp>(loc, source, temp, temp.isAllocatable(),
- /*keep_lhs_length_if_realloc=*/false,
- /*temporary_lhs=*/true);
- mlir::Value bufferizedExpr =
- packageBufferizedExpr(loc, builder, temp, cleanup);
+ mlir::Value bufferizedExpr = copyInTempAndPackage(loc, builder, source);
rewriter.replaceOp(asExpr, bufferizedExpr);
return mlir::success();
}
@@ -542,12 +553,7 @@ struct AssociateOpConversion
// non-trivial value with more than one use. We will have to make a copy and
// use that
hlfir::Entity source = hlfir::Entity{bufferizedExpr};
- auto [temp, cleanup] = hlfir::createTempFromMold(loc, builder, source);
- builder.create<hlfir::AssignOp>(loc, source, temp, temp.isAllocatable(),
- /*keep_lhs_length_if_realloc=*/false,
- /*temporary_lhs=*/true);
- mlir::Value bufferTuple =
- packageBufferizedExpr(loc, builder, temp, cleanup);
+ mlir::Value bufferTuple = copyInTempAndPackage(loc, builder, source);
bufferizedExpr = getBufferizedExprStorage(bufferTuple);
replaceWith(bufferizedExpr, hlfir::Entity{bufferizedExpr}.getFirBase(),
getBufferizedExprMustFreeFlag(bufferTuple));
diff --git a/flang/test/HLFIR/as_expr-codegen-polymorphic.fir b/flang/test/HLFIR/as_expr-codegen-polymorphic.fir
new file mode 100644
index 0000000000000..30a8da72c3e48
--- /dev/null
+++ b/flang/test/HLFIR/as_expr-codegen-polymorphic.fir
@@ -0,0 +1,38 @@
+// Test hlfir.as_expr codegen for polymorphic expressions.
+
+// RUN: fir-opt %s -bufferize-hlfir | FileCheck %s
+
+!t = !fir.type<t{i:i32}>
+func.func @as_expr_class(%arg0 : !fir.class<!t>, %arg1: !fir.ref<!t>) {
+ %0 = hlfir.as_expr %arg0 : (!fir.class<!t>) -> !hlfir.expr<!t?>
+ hlfir.assign %0 to %arg1 : !hlfir.expr<!t?>, !fir.ref<!t>
+ return
+}
+// CHECK-LABEL: func.func @as_expr_class(
+// CHECK: %[[VAL_5:.*]] = arith.constant true
+// CHECK: %[[VAL_6:.*]]:2 = hlfir.declare %{{.*}} {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = ".tmp"} : (!fir.ref<!fir.class<!fir.heap<!fir.type<t{i:i32}>>>>) -> (!fir.ref<!fir.class<!fir.heap<!fir.type<t{i:i32}>>>>, !fir.ref<!fir.class<!fir.heap<!fir.type<t{i:i32}>>>>)
+// ... copy ...
+// CHECK: %[[VAL_11:.*]] = fir.load %[[VAL_6]]#0 : !fir.ref<!fir.class<!fir.heap<!fir.type<t{i:i32}>>>>
+// CHECK: %[[VAL_12:.*]] = fir.undefined tuple<!fir.class<!fir.heap<!fir.type<t{i:i32}>>>, i1>
+// CHECK: %[[VAL_13:.*]] = fir.insert_value %[[VAL_12]], %[[VAL_5]], [1 : index] : (tuple<!fir.class<!fir.heap<!fir.type<t{i:i32}>>>, i1>, i1) -> tuple<!fir.class<!fir.heap<!fir.type<t{i:i32}>>>, i1>
+// CHECK: %[[VAL_14:.*]] = fir.insert_value %[[VAL_13]], %[[VAL_11]], [0 : index] : (tuple<!fir.class<!fir.heap<!fir.type<t{i:i32}>>>, i1>, !fir.class<!fir.heap<!fir.type<t{i:i32}>>>) -> tuple<!fir.class<!fir.heap<!fir.type<t{i:i32}>>>, i1>
+// CHECK: hlfir.assign %[[VAL_11]] to %{{.*}} : !fir.class<!fir.heap<!fir.type<t{i:i32}>>>, !fir.ref<!fir.type<t{i:i32}>>
+
+
+func.func @as_expr_class_2(%arg0 : !fir.class<!fir.array<?x!t>>) {
+ %0 = hlfir.as_expr %arg0 : (!fir.class<!fir.array<?x!t>>) -> !hlfir.expr<?x!t?>
+ %c1 = arith.constant 1 : index
+ %1 = hlfir.apply %0, %c1 : (!hlfir.expr<?x!t?>, index) -> !hlfir.expr<!t?>
+ return
+}
+// CHECK-LABEL: func.func @as_expr_class_2(
+// CHECK: %[[VAL_9:.*]] = arith.constant true
+// CHECK: %[[VAL_10:.*]]:2 = hlfir.declare %{{.*}} {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = ".tmp"} : (!fir.ref<!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>>) -> (!fir.ref<!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>>, !fir.ref<!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>>)
+// CHECK: %[[VAL_11:.*]] = arith.constant 1 : i32
+// ... copy ...
+// CHECK: %[[VAL_15:.*]] = fir.load %[[VAL_10]]#0 : !fir.ref<!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>>
+// CHECK: %[[VAL_16:.*]] = fir.undefined tuple<!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>, i1>
+// CHECK: %[[VAL_17:.*]] = fir.insert_value %[[VAL_16]], %[[VAL_9]], [1 : index] : (tuple<!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>, i1>, i1) -> tuple<!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>, i1>
+// CHECK: %[[VAL_18:.*]] = fir.insert_value %[[VAL_17]], %[[VAL_15]], [0 : index] : (tuple<!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>, i1>, !fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>) -> tuple<!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>, i1>
+// CHECK: %[[VAL_19:.*]] = arith.constant 1 : index
+// CHECK: %[[VAL_20:.*]] = hlfir.designate %[[VAL_15]] (%[[VAL_19]]) : (!fir.class<!fir.heap<!fir.array<?x!fir.type<t{i:i32}>>>>, index) -> !fir.class<!fir.type<t{i:i32}>>
diff --git a/flang/test/HLFIR/bufferize-poly-expr.fir b/flang/test/HLFIR/bufferize-poly-expr.fir
index df43c74b2aacc..dfa62a9ac5ab7 100644
--- a/flang/test/HLFIR/bufferize-poly-expr.fir
+++ b/flang/test/HLFIR/bufferize-poly-expr.fir
@@ -26,12 +26,12 @@ func.func @test_poly_expr_without_associate() {
// CHECK: %[[VAL_11:.*]] = fir.convert %[[VAL_4]]#1 : (!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>) -> !fir.box<none>
// CHECK: %[[VAL_12:.*]] = fir.call @_FortranAAllocatableApplyMold(%[[VAL_10]], %[[VAL_11]], %[[VAL_9]]) : (!fir.ref<!fir.box<none>>, !fir.box<none>, i32) -> none
// CHECK: hlfir.assign %[[VAL_4]]#0 to %[[VAL_8]]#0 realloc temporary_lhs : !fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>, !fir.ref<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>>
-// CHECK: %[[VAL_13:.*]] = fir.undefined tuple<!fir.ref<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>>, i1>
-// CHECK: %[[VAL_14:.*]] = fir.insert_value %[[VAL_13]], %[[VAL_7]], [1 : index] : (tuple<!fir.ref<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>>, i1>, i1) -> tuple<!fir.ref<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>>, i1>
-// CHECK: %[[VAL_15:.*]] = fir.insert_value %[[VAL_14]], %[[VAL_8]]#0, [0 : index] : (tuple<!fir.ref<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>>, i1>, !fir.ref<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>>) -> tuple<!fir.ref<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>>, i1>
-// CHECK: hlfir.assign %[[VAL_8]]#0 to %[[VAL_2]]#0 realloc : !fir.ref<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>>, !fir.ref<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>>
-// CHECK: %[[VAL_16:.*]] = fir.load %[[VAL_8]]#0 : !fir.ref<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>>
-// CHECK: %[[VAL_17:.*]] = fir.box_addr %[[VAL_16]] : (!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>) -> !fir.heap<!fir.type<_QFtestTt{c:i32}>>
+// CHECK: %[[VAL_8B:.*]] = fir.load %[[VAL_8]]#0
+// CHECK: %[[VAL_13:.*]] = fir.undefined tuple<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>, i1>
+// CHECK: %[[VAL_14:.*]] = fir.insert_value %[[VAL_13]], %[[VAL_7]], [1 : index] : (tuple<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>, i1>, i1) -> tuple<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>, i1>
+// CHECK: %[[VAL_15:.*]] = fir.insert_value %[[VAL_14]], %[[VAL_8B]], [0 : index] : (tuple<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>, i1>, !fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>) -> tuple<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>, i1>
+// CHECK: hlfir.assign %[[VAL_8B]] to %[[VAL_2]]#0 realloc : !fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>, !fir.ref<!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>>
+// CHECK: %[[VAL_17:.*]] = fir.box_addr %[[VAL_8B]] : (!fir.class<!fir.heap<!fir.type<_QFtestTt{c:i32}>>>) -> !fir.heap<!fir.type<_QFtestTt{c:i32}>>
// CHECK: fir.freemem %[[VAL_17]] : !fir.heap<!fir.type<_QFtestTt{c:i32}>>
// CHECK: return
// CHECK: }
@@ -81,14 +81,13 @@ func.func @test_poly_expr_with_associate(%arg1: !fir.class<!fir.array<3x!fir.typ
// CHECK: %[[VAL_17:.*]] = fir.convert %[[VAL_5]] : (!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>) -> !fir.box<none>
// CHECK: %[[VAL_18:.*]] = fir.call @_FortranAAllocatableApplyMold(%[[VAL_16]], %[[VAL_17]], %[[VAL_15]]) : (!fir.ref<!fir.box<none>>, !fir.box<none>, i32) -> none
// CHECK: hlfir.assign %[[VAL_5]] to %[[VAL_14]]#0 realloc temporary_lhs : !fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>, !fir.ref<!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>>
-// CHECK: %[[VAL_19:.*]] = fir.undefined tuple<!fir.ref<!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>>, i1>
-// CHECK: %[[VAL_20:.*]] = fir.insert_value %[[VAL_19]], %[[VAL_13]], [1 : index] : (tuple<!fir.ref<!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>>, i1>, i1) -> tuple<!fir.ref<!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>>, i1>
-// CHECK: %[[VAL_21:.*]] = fir.insert_value %[[VAL_20]], %[[VAL_14]]#0, [0 : index] : (tuple<!fir.ref<!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>>, i1>, !fir.ref<!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>>) -> tuple<!fir.ref<!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>>, i1>
+// CHECK: %[[VAL_14B:.*]] = fir.load %[[VAL_14]]#0
+// CHECK: %[[VAL_19:.*]] = fir.undefined tuple<!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>, i1>
+// CHECK: %[[VAL_20:.*]] = fir.insert_value %[[VAL_19]], %[[VAL_13]], [1 : index] : (tuple<!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>, i1>, i1) -> tuple<!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>, i1>
+// CHECK: %[[VAL_21:.*]] = fir.insert_value %[[VAL_20]], %[[VAL_14B]], [0 : index] : (tuple<!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>, i1>, !fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>) -> tuple<!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>, i1>
// CHECK: %[[VAL_22:.*]] = arith.constant 0 : index
// CHECK: %[[VAL_23:.*]]:3 = fir.box_dims %[[VAL_5]], %[[VAL_22]] : (!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>, index) -> (index, index, index)
// CHECK: %[[VAL_24:.*]] = fir.shape %[[VAL_23]]#1 : (index) -> !fir.shape<1>
-// CHECK: %[[VAL_25:.*]] = fir.load %[[VAL_14]]#0 : !fir.ref<!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>>
-// CHECK: %[[VAL_26:.*]] = fir.load %[[VAL_14]]#1 : !fir.ref<!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>>
// CHECK: %[[VAL_27:.*]] = fir.convert %[[VAL_2]] : (!fir.ref<!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>>) -> !fir.box<none>
// CHECK: %[[VAL_28:.*]] = fir.call @_FortranADestroy(%[[VAL_27]]) fastmath<contract> : (!fir.box<none>) -> none
// CHECK: %[[VAL_29:.*]] = arith.constant 3 : index
@@ -96,10 +95,10 @@ func.func @test_poly_expr_with_associate(%arg1: !fir.class<!fir.array<3x!fir.typ
// CHECK: %[[VAL_31:.*]] = arith.constant 1 : index
// CHECK: fir.do_loop %[[VAL_32:.*]] = %[[VAL_31]] to %[[VAL_29]] step %[[VAL_31]] {
// CHECK: %[[VAL_33:.*]] = hlfir.designate %[[VAL_3]]#0 (%[[VAL_32]]) : (!fir.class<!fir.array<3x!fir.type<_QMtest_typeTt1{i:i32}>>>, index) -> !fir.class<!fir.type<_QMtest_typeTt1{i:i32}>>
-// CHECK: %[[VAL_34:.*]] = hlfir.designate %[[VAL_25]] (%[[VAL_32]]) : (!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>, index) -> !fir.class<!fir.type<_QMtest_typeTt1{i:i32}>>
+// CHECK: %[[VAL_34:.*]] = hlfir.designate %[[VAL_14B]] (%[[VAL_32]]) : (!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>, index) -> !fir.class<!fir.type<_QMtest_typeTt1{i:i32}>>
// CHECK: fir.dispatch "assign"(%[[VAL_33]] : !fir.class<!fir.type<_QMtest_typeTt1{i:i32}>>) (%[[VAL_33]], %[[VAL_34]] : !fir.class<!fir.type<_QMtest_typeTt1{i:i32}>>, !fir.class<!fir.type<_QMtest_typeTt1{i:i32}>>) {pass_arg_pos = 0 : i32}
// CHECK: }
-// CHECK: %[[VAL_35:.*]] = fir.box_addr %[[VAL_26]] : (!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>) -> !fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>
+// CHECK: %[[VAL_35:.*]] = fir.box_addr %[[VAL_14B]] : (!fir.class<!fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>>) -> !fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>
// CHECK: fir.freemem %[[VAL_35]] : !fir.heap<!fir.array<?x!fir.type<_QMtest_typeTt1{i:i32}>>>
// CHECK: return
// CHECK: }
More information about the flang-commits
mailing list