From 18047482714d6450f09b989b1ee36cf490402c61 Mon Sep 17 00:00:00 2001 From: Jean Perier Date: Mon, 9 Dec 2024 06:55:56 -0800 Subject: [PATCH 1/2] [flang][hlfir] fix issue 118922 --- .../HLFIR/Transforms/BufferizeHLFIR.cpp | 9 +++- .../HLFIR/element-codegen-issue-118922.fir | 48 +++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 flang/test/HLFIR/element-codegen-issue-118922.fir diff --git a/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp index 347f0a5630777..88d20b7bee2a0 100644 --- a/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp +++ b/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp @@ -806,8 +806,13 @@ struct ElementalOpConversion // elemental is a "view" over a variable (e.g parentheses or transpose). if (auto asExpr = elementValue.getDefiningOp()) { if (asExpr->hasOneUse() && !asExpr.isMove()) { - elementValue = hlfir::Entity{asExpr.getVar()}; - rewriter.eraseOp(asExpr); + // Check that the asExpr is the final operation before the yield, + // otherwise, clean-ups could impact the memory being re-used. + mlir::Operation *nextOp = asExpr->getNextNode(); + if (nextOp && nextOp == yield.getOperation()) { + elementValue = hlfir::Entity{asExpr.getVar()}; + rewriter.eraseOp(asExpr); + } } } rewriter.eraseOp(yield); diff --git a/flang/test/HLFIR/element-codegen-issue-118922.fir b/flang/test/HLFIR/element-codegen-issue-118922.fir new file mode 100644 index 0000000000000..8fc0aa92a6b35 --- /dev/null +++ b/flang/test/HLFIR/element-codegen-issue-118922.fir @@ -0,0 +1,48 @@ +// Test issue 113843 and 118922 fix: do not elide hlfir.elemental final as_expr +// copy if this is not the last operation. +// RUN: fir-opt %s --bufferize-hlfir | FileCheck %s + +func.func @_QMmPbug(%val: !fir.char<1>, %var: !fir.ref>>) { + %c1 = arith.constant 1 : index + %c10 = arith.constant 10 : index + %1 = fir.shape %c10 : (index) -> !fir.shape<1> + %expr = hlfir.elemental %1 typeparams %c1 unordered : (!fir.shape<1>, index) -> !hlfir.expr<10x!fir.char<1>> { + ^bb0(%arg2: index): + %alloc = fir.allocmem !fir.char<1> + fir.store %val to %alloc : !fir.heap> + %addr = fir.convert %alloc: (!fir.heap>) -> !fir.ref> + %9 = hlfir.as_expr %addr : (!fir.ref>) -> !hlfir.expr> + fir.freemem %alloc : !fir.heap> + hlfir.yield_element %9 : !hlfir.expr> + } + hlfir.assign %expr to %var : !hlfir.expr<10x!fir.char<1>>, !fir.ref>> + hlfir.destroy %expr : !hlfir.expr<10x!fir.char<1>> + return +} + +// CHECK-LABEL: func.func @_QMmPbug( +// CHECK-SAME: %[[VAL_0:.*]]: !fir.char<1>, +// CHECK-SAME: %[[VAL_1:.*]]: !fir.ref>>) { +// CHECK: %[[VAL_2:.*]] = fir.alloca !fir.char<1> {bindc_name = ".tmp"} +// CHECK: %[[VAL_3:.*]] = arith.constant 1 : index +// CHECK: %[[VAL_4:.*]] = arith.constant 10 : index +// CHECK: %[[VAL_5:.*]] = fir.shape %[[VAL_4]] : (index) -> !fir.shape<1> +// CHECK: %[[VAL_6:.*]] = fir.allocmem !fir.array<10x!fir.char<1>> {bindc_name = ".tmp.array", uniq_name = ""} +// CHECK: %[[VAL_7:.*]]:2 = hlfir.declare %[[VAL_6]](%[[VAL_5]]) typeparams %[[VAL_3]] {uniq_name = ".tmp.array"} : (!fir.heap>>, !fir.shape<1>, index) -> (!fir.heap>>, !fir.heap>>) +// CHECK: %[[VAL_8:.*]] = arith.constant true +// CHECK: %[[VAL_9:.*]] = arith.constant 1 : index +// CHECK: fir.do_loop %[[VAL_10:.*]] = %[[VAL_9]] to %[[VAL_4]] step %[[VAL_9]] unordered { +// CHECK: %[[VAL_11:.*]] = fir.allocmem !fir.char<1> +// CHECK: fir.store %[[VAL_0]] to %[[VAL_11]] : !fir.heap> +// CHECK: %[[VAL_12:.*]] = fir.convert %[[VAL_11]] : (!fir.heap>) -> !fir.ref> +// CHECK: %[[VAL_13:.*]] = arith.constant 1 : index +// CHECK: %[[VAL_14:.*]] = arith.constant false +// CHECK: %[[VAL_15:.*]]:2 = hlfir.declare %[[VAL_2]] typeparams %[[VAL_13]] {uniq_name = ".tmp"} : (!fir.ref>, index) -> (!fir.ref>, !fir.ref>) +// CHECK: hlfir.assign %[[VAL_12]] to %[[VAL_15]]#0 temporary_lhs : !fir.ref>, !fir.ref> +// CHECK: %[[VAL_16:.*]] = fir.undefined tuple>, i1> +// CHECK: %[[VAL_17:.*]] = fir.insert_value %[[VAL_16]], %[[VAL_14]], [1 : index] : (tuple>, i1>, i1) -> tuple>, i1> +// CHECK: %[[VAL_18:.*]] = fir.insert_value %[[VAL_17]], %[[VAL_15]]#0, [0 : index] : (tuple>, i1>, !fir.ref>) -> tuple>, i1> +// CHECK: fir.freemem %[[VAL_11]] : !fir.heap> +// CHECK: %[[VAL_19:.*]] = hlfir.designate %[[VAL_7]]#0 (%[[VAL_10]]) typeparams %[[VAL_3]] : (!fir.heap>>, index, index) -> !fir.ref> +// CHECK: hlfir.assign %[[VAL_15]]#0 to %[[VAL_19]] temporary_lhs : !fir.ref>, !fir.ref> +// CHECK: } From 4d6f72f75ad954ccb1530e66fc232f1f54eed8e9 Mon Sep 17 00:00:00 2001 From: Jean Perier Date: Tue, 10 Dec 2024 00:23:42 -0800 Subject: [PATCH 2/2] simplify check --- flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp index 88d20b7bee2a0..48e3db0eb39b6 100644 --- a/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp +++ b/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp @@ -808,8 +808,7 @@ struct ElementalOpConversion if (asExpr->hasOneUse() && !asExpr.isMove()) { // Check that the asExpr is the final operation before the yield, // otherwise, clean-ups could impact the memory being re-used. - mlir::Operation *nextOp = asExpr->getNextNode(); - if (nextOp && nextOp == yield.getOperation()) { + if (asExpr->getNextNode() == yield.getOperation()) { elementValue = hlfir::Entity{asExpr.getVar()}; rewriter.eraseOp(asExpr); }