Skip to content

Commit b9e435c

Browse files
committed
[flang][hlfir] Removed incorrect clean-up in the implied-do lowering.
The lowering of calls/expressions unconditionally inserts DestroyOp clean-up for hlfir.expr values, which is wrong in the case where the value is used as a result of the elemental operation created during the implied-do lowering. A cleaner fix could be to avoid DestroyOp insertion at all, but I have not figure out an easy way to do it. The DestroyOp look-up I used seems to be quite reliable, so it should just work. Reviewed By: clementval Differential Revision: https://reviews.llvm.org/D155665
1 parent b636987 commit b9e435c

File tree

3 files changed

+50
-1
lines changed

3 files changed

+50
-1
lines changed

flang/include/flang/Optimizer/HLFIR/HLFIROps.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -894,7 +894,7 @@ def hlfir_DestroyOp : hlfir_Op<"destroy", [MemoryEffects<[MemFree]>]> {
894894
buffer of an hlfir.expr, if any, will be deallocated if it was heap
895895
allocated.
896896
It is not required to create an hlfir.destroy operation for and hlfir.expr
897-
created inside an hlfir.elemental an returned in the hlfir.yield_element.
897+
created inside an hlfir.elemental and returned in the hlfir.yield_element.
898898
The last use of such expression is implicit and an hlfir.destroy could
899899
not be emitted after the hlfir.yield_element since it is a terminator.
900900

flang/lib/Lower/ConvertArrayConstructor.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,26 @@ class AsElementalStrategy : public StrategyBase {
241241
// right now.
242242
stmtCtx.finalizeAndPop();
243243

244+
// This is a hacky way to get rid of the DestroyOp clean-up
245+
// associated with the final ac-value result if it is hlfir.expr.
246+
// Example:
247+
// ... = (/(REPEAT(REPEAT(CHAR(i),2),2),i=1,n)/)
248+
// Each intrinsic call lowering will produce hlfir.expr result
249+
// with the associated clean-up, but only the last of them
250+
// is wrong. It is wrong because the value is used in hlfir.yield_element,
251+
// so it cannot be destroyed.
252+
mlir::Operation *destroyOp = nullptr;
253+
for (mlir::Operation *useOp : elementResult.getUsers())
254+
if (mlir::isa<hlfir::DestroyOp>(useOp)) {
255+
if (destroyOp)
256+
fir::emitFatalError(loc,
257+
"multiple DestroyOp's for ac-value expression");
258+
destroyOp = useOp;
259+
}
260+
261+
if (destroyOp)
262+
destroyOp->erase();
263+
244264
builder.create<hlfir::YieldElementOp>(loc, elementResult);
245265
}
246266

flang/test/Lower/HLFIR/array-ctor-as-elemental.f90

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,3 +128,32 @@ integer function impure_foo(i)
128128
end subroutine
129129
! CHECK-NOT: hlfir.elemental
130130
! CHECK: return
131+
132+
! Test that the hlfir.expr result of the outer intrinsic call
133+
! is not destructed.
134+
subroutine test_hlfir_expr_result_destruction
135+
character(4) :: a(21)
136+
a = (/ (repeat(repeat(char(i),2),2),i=1,n) /)
137+
end subroutine
138+
! CHECK-LABEL: func.func @_QPtest_hlfir_expr_result_destruction() {
139+
! CHECK: %[[VAL_36:.*]] = hlfir.elemental %{{.*}} typeparams %{{.*}} unordered : (!fir.shape<1>, index) -> !hlfir.expr<?x!fir.char<1,?>> {
140+
! CHECK: %[[VAL_48:.*]] = hlfir.as_expr %{{.*}} move %{{.*}} : (!fir.ref<!fir.char<1>>, i1) -> !hlfir.expr<!fir.char<1>>
141+
! CHECK: %[[VAL_51:.*]]:3 = hlfir.associate %[[VAL_48]] typeparams %{{.*}} {uniq_name = "adapt.valuebyref"} : (!hlfir.expr<!fir.char<1>>, index) -> (!fir.ref<!fir.char<1>>, !fir.ref<!fir.char<1>>, i1)
142+
! CHECK: %[[VAL_64:.*]]:2 = hlfir.declare %{{.*}} typeparams %{{.*}} {uniq_name = ".tmp.intrinsic_result"} : (!fir.heap<!fir.char<1,2>>, index) -> (!fir.heap<!fir.char<1,2>>, !fir.heap<!fir.char<1,2>>)
143+
! CHECK: %[[VAL_66:.*]] = hlfir.as_expr %[[VAL_64]]#0 move %{{.*}} : (!fir.heap<!fir.char<1,2>>, i1) -> !hlfir.expr<!fir.char<1,2>>
144+
! CHECK: %[[VAL_68:.*]]:3 = hlfir.associate %[[VAL_66]] typeparams %{{.*}} {uniq_name = "adapt.valuebyref"} : (!hlfir.expr<!fir.char<1,2>>, index) -> (!fir.ref<!fir.char<1,2>>, !fir.ref<!fir.char<1,2>>, i1)
145+
! CHECK: %[[VAL_81:.*]]:2 = hlfir.declare %{{.*}} typeparams %{{.*}} {uniq_name = ".tmp.intrinsic_result"} : (!fir.heap<!fir.char<1,4>>, index) -> (!fir.heap<!fir.char<1,4>>, !fir.heap<!fir.char<1,4>>)
146+
! CHECK: %[[VAL_83:.*]] = hlfir.as_expr %[[VAL_81]]#0 move %{{.*}} : (!fir.heap<!fir.char<1,4>>, i1) -> !hlfir.expr<!fir.char<1,4>>
147+
! CHECK-NOT: hlfir.destroy %[[VAL_83]]
148+
! CHECK: hlfir.end_associate %[[VAL_68]]#1, %[[VAL_68]]#2 : !fir.ref<!fir.char<1,2>>, i1
149+
! CHECK-NOT: hlfir.destroy %[[VAL_83]]
150+
! CHECK: hlfir.destroy %[[VAL_66]] : !hlfir.expr<!fir.char<1,2>>
151+
! CHECK-NOT: hlfir.destroy %[[VAL_83]]
152+
! CHECK: hlfir.end_associate %[[VAL_51]]#1, %[[VAL_51]]#2 : !fir.ref<!fir.char<1>>, i1
153+
! CHECK-NOT: hlfir.destroy %[[VAL_83]]
154+
! CHECK: hlfir.destroy %[[VAL_48]] : !hlfir.expr<!fir.char<1>>
155+
! CHECK-NOT: hlfir.destroy %[[VAL_83]]
156+
! CHECK: hlfir.yield_element %[[VAL_83]] : !hlfir.expr<!fir.char<1,4>>
157+
! CHECK-NOT: hlfir.destroy %[[VAL_83]]
158+
! CHECK: }
159+
! CHECK-NOT: hlfir.destroy %[[VAL_83]]

0 commit comments

Comments
 (0)