Skip to content

[flang][hlfir] Return function call result as AsExpr. #67769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 2, 2023
Merged

[flang][hlfir] Return function call result as AsExpr. #67769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e27bda
[flang][hlfir] Return function call result as AsExpr.
vzakhari Sep 29, 2023
9bd6b5e
Moved deref to lowering.
vzakhari Sep 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions flang/lib/Lower/ConvertCall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1232,8 +1232,25 @@ genUserCall(Fortran::lower::PreparedActualArguments &loweredActuals,

if (!fir::getBase(result))
return std::nullopt; // subroutine call.
// TODO: "move" non pointer results into hlfir.expr.
return extendedValueToHlfirEntity(loc, builder, result, ".tmp.func_result");

hlfir::Entity resultEntity =
extendedValueToHlfirEntity(loc, builder, result, ".tmp.func_result");

if (!fir::isPointerType(fir::getBase(result).getType())) {
resultEntity = loadTrivialScalar(loc, builder, resultEntity);

if (resultEntity.isVariable()) {
// Function result must not be freed, since it is allocated on the stack.
Copy link
Contributor

@tblah tblah Sep 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about hlfir.destroy? I think in other cases we do create a destroy operation even when the result does not need deallocation. But I am open to not doing so.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be possible to use hlfir.destroy, though, currently we destroy the function result as an in-memory entity (e.g. see #67768). It may even be a potential correctness issue currently, e.g. if the as_expr is moved along with its use past the destroy call. @jeanPerier, do you think we need to make the buffer live-range explicit here with hlfir.as_exp->hlfir.destroy def-use?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tom, it makes sense to use hlfir.destroy for the clean-up of the non-elemental result. I will do it, when we get rid of the FIR path. I hope using as_expr now should not result in incorrect MLIR optimizations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may even be a potential correctness issue currently, e.g. if the as_expr is moved along with its use past the destroy call. @jeanPerier, do you think we need to make the buffer live-range explicit here with hlfir.as_exp->hlfir.destroy def-use?

Yes, it would be cleaner. As you mentioned, the clean-up is currently done on the memory that was available before it is moved, this probably does not cause any issue currently, but could if the life of hlfir.expr is prolongated for whatever reasons. I agree the change will be easier without having to deal with both FIR and HLFIR.

// Note that in non-elemental case, genCallOpAndResult()
// is responsible for establishing the clean-up that destroys
// the derived type result or deallocates its components
// without finalization.
auto asExpr = builder.create<hlfir::AsExprOp>(
loc, resultEntity, /*mustFree=*/builder.createBool(loc, false));
resultEntity = hlfir::EntityWithAttributes{asExpr.getResult()};
}
}
return hlfir::EntityWithAttributes{resultEntity};
}

/// Create an optional dummy argument value from an entity that may be
Expand Down
4 changes: 3 additions & 1 deletion flang/test/Lower/HLFIR/elemental-array-ops.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,10 @@ end subroutine char_return
! CHECK: %[[VAL_26:.*]] = fir.call @llvm.stacksave.p0() fastmath<contract> : () -> !fir.ref<i8>
! CHECK: %[[VAL_27:.*]] = fir.call @_QPcallee(%[[VAL_2]], %[[VAL_25]], %[[VAL_20]]) fastmath<contract> : (!fir.ref<!fir.char<1,3>>, index, !fir.boxchar<1>) -> !fir.boxchar<1>
! CHECK: %[[VAL_28:.*]]:2 = hlfir.declare %[[VAL_2]] typeparams %[[VAL_25]] {uniq_name = ".tmp.func_result"} : (!fir.ref<!fir.char<1,3>>, index) -> (!fir.ref<!fir.char<1,3>>, !fir.ref<!fir.char<1,3>>)
! CHECK: %[[MustFree:.*]] = arith.constant false
! CHECK: %[[ResultTemp:.*]] = hlfir.as_expr %[[VAL_28]]#0 move %[[MustFree]] : (!fir.ref<!fir.char<1,3>>, i1) -> !hlfir.expr<!fir.char<1,3>>
! CHECK: fir.call @llvm.stackrestore.p0(%[[VAL_26]]) fastmath<contract> : (!fir.ref<i8>) -> ()
! CHECK: hlfir.yield_element %[[VAL_28]]#0 : !fir.ref<!fir.char<1,3>>
! CHECK: hlfir.yield_element %[[ResultTemp]] : !hlfir.expr<!fir.char<1,3>>
! CHECK: }
! CHECK: %[[VAL_29:.*]] = arith.constant 0 : index
! CHECK: %[[VAL_30:.*]]:3 = fir.box_dims %[[VAL_10]]#0, %[[VAL_29]] : (!fir.box<!fir.array<?x!fir.char<1,3>>>, index) -> (index, index, index)
Expand Down
135 changes: 135 additions & 0 deletions flang/test/Lower/HLFIR/function-return-as-expr.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
! RUN: bbc -emit-hlfir --polymorphic-type -o - %s -I nowhere 2>&1 | FileCheck %s

module types
type t1
end type t1
end module types

subroutine test1
integer :: i
i = inner() + 1
contains
function inner()
integer, allocatable :: inner
end function inner
end subroutine test1
! CHECK-LABEL: func.func @_QPtest1() {
! CHECK: %[[VAL_0:.*]] = fir.alloca !fir.box<!fir.heap<i32>> {bindc_name = ".result"}
! CHECK: %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_0]] {uniq_name = ".tmp.func_result"} : (!fir.ref<!fir.box<!fir.heap<i32>>>) -> (!fir.ref<!fir.box<!fir.heap<i32>>>, !fir.ref<!fir.box<!fir.heap<i32>>>)
! CHECK: %[[VAL_5:.*]] = fir.load %[[VAL_4]]#0 : !fir.ref<!fir.box<!fir.heap<i32>>>
! CHECK: %[[VAL_6:.*]] = fir.box_addr %[[VAL_5]] : (!fir.box<!fir.heap<i32>>) -> !fir.heap<i32>
! CHECK: %[[VAL_7:.*]] = fir.load %[[VAL_6]] : !fir.heap<i32>
! CHECK: %[[VAL_8:.*]] = arith.constant 1 : i32
! CHECK: %[[VAL_9:.*]] = arith.addi %[[VAL_7]], %[[VAL_8]] : i32

subroutine test2
character(len=:), allocatable :: c
c = inner()
contains
function inner()
character(len=:), allocatable :: inner
end function inner
end subroutine test2
! CHECK-LABEL: func.func @_QPtest2() {
! CHECK: %[[VAL_7:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = ".tmp.func_result"} : (!fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>) -> (!fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>, !fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>)
! CHECK: %[[VAL_8:.*]] = fir.load %[[VAL_7]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>
! CHECK: %[[VAL_9:.*]] = fir.box_addr %[[VAL_8]] : (!fir.box<!fir.heap<!fir.char<1,?>>>) -> !fir.heap<!fir.char<1,?>>
! CHECK: %[[VAL_10:.*]] = fir.load %[[VAL_7]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>
! CHECK: %[[VAL_11:.*]] = fir.box_elesize %[[VAL_10]] : (!fir.box<!fir.heap<!fir.char<1,?>>>) -> index
! CHECK: %[[VAL_12:.*]] = fir.emboxchar %[[VAL_9]], %[[VAL_11]] : (!fir.heap<!fir.char<1,?>>, index) -> !fir.boxchar<1>
! CHECK: %[[VAL_13:.*]] = arith.constant false
! CHECK: %[[VAL_14:.*]] = hlfir.as_expr %[[VAL_12]] move %[[VAL_13]] : (!fir.boxchar<1>, i1) -> !hlfir.expr<!fir.char<1,?>>
! CHECK: hlfir.assign %[[VAL_14]] to %{{.*}}#0 realloc : !hlfir.expr<!fir.char<1,?>>, !fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>

subroutine test3
character(len=:), allocatable :: c
c = inner()
contains
function inner()
character(len=3), allocatable :: inner
end function inner
end subroutine test3
! CHECK-LABEL: func.func @_QPtest3() {
! CHECK: %[[VAL_13:.*]]:2 = hlfir.declare %{{.*}} typeparams %{{.*}} {uniq_name = ".tmp.func_result"} : (!fir.ref<!fir.box<!fir.heap<!fir.char<1,3>>>>, index) -> (!fir.ref<!fir.box<!fir.heap<!fir.char<1,3>>>>, !fir.ref<!fir.box<!fir.heap<!fir.char<1,3>>>>)
! CHECK: %[[VAL_14:.*]] = fir.load %[[VAL_13]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.char<1,3>>>>
! CHECK: %[[VAL_15:.*]] = fir.box_addr %[[VAL_14]] : (!fir.box<!fir.heap<!fir.char<1,3>>>) -> !fir.heap<!fir.char<1,3>>
! CHECK: %[[VAL_16:.*]] = arith.constant false
! CHECK: %[[VAL_17:.*]] = hlfir.as_expr %[[VAL_15]] move %[[VAL_16]] : (!fir.heap<!fir.char<1,3>>, i1) -> !hlfir.expr<!fir.char<1,3>>
! CHECK: hlfir.assign %[[VAL_17]] to %{{.*}}#0 realloc : !hlfir.expr<!fir.char<1,3>>, !fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>

subroutine test4
class(*), allocatable :: p
p = inner()
contains
function inner()
class(*), allocatable :: inner
end function inner
end subroutine test4
! CHECK-LABEL: func.func @_QPtest4() {
! CHECK: %[[VAL_6:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = ".tmp.func_result"} : (!fir.ref<!fir.class<!fir.heap<none>>>) -> (!fir.ref<!fir.class<!fir.heap<none>>>, !fir.ref<!fir.class<!fir.heap<none>>>)
! CHECK: %[[VAL_7:.*]] = fir.load %[[VAL_6]]#0 : !fir.ref<!fir.class<!fir.heap<none>>>
! CHECK: %[[VAL_8:.*]] = arith.constant false
! CHECK: %[[VAL_9:.*]] = hlfir.as_expr %[[VAL_7]] move %[[VAL_8]] : (!fir.class<!fir.heap<none>>, i1) -> !hlfir.expr<none?>
! CHECK: hlfir.assign %[[VAL_9]] to %{{.*}}#0 realloc : !hlfir.expr<none?>, !fir.ref<!fir.class<!fir.heap<none>>>

subroutine test5
use types
type(t1) :: r
r = inner()
contains
function inner()
type(t1) :: inner
end function inner
end subroutine test5
! CHECK-LABEL: func.func @_QPtest5() {
! CHECK: %[[VAL_4:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = ".tmp.func_result"} : (!fir.ref<!fir.type<_QMtypesTt1>>) -> (!fir.ref<!fir.type<_QMtypesTt1>>, !fir.ref<!fir.type<_QMtypesTt1>>)
! CHECK: %[[VAL_5:.*]] = arith.constant false
! CHECK: %[[VAL_6:.*]] = hlfir.as_expr %[[VAL_4]]#0 move %[[VAL_5]] : (!fir.ref<!fir.type<_QMtypesTt1>>, i1) -> !hlfir.expr<!fir.type<_QMtypesTt1>>
! CHECK: hlfir.assign %[[VAL_6]] to %{{.*}}#0 : !hlfir.expr<!fir.type<_QMtypesTt1>>, !fir.ref<!fir.type<_QMtypesTt1>>

subroutine test6(x)
character(len=:), allocatable :: c(:)
integer :: x(:)
c = inner(x)
contains
elemental function inner(x)
integer, intent(in) :: x
character(len=3) :: inner
end function inner
end subroutine test6
! CHECK-LABEL: func.func @_QPtest6(
! CHECK: %[[VAL_14:.*]] = hlfir.elemental
! CHECK: %[[VAL_24:.*]]:2 = hlfir.declare %{{.*}} typeparams %{{.*}} {uniq_name = ".tmp.func_result"} : (!fir.ref<!fir.char<1,3>>, index) -> (!fir.ref<!fir.char<1,3>>, !fir.ref<!fir.char<1,3>>)
! CHECK: %[[VAL_25:.*]] = arith.constant false
! CHECK: %[[VAL_26:.*]] = hlfir.as_expr %[[VAL_24]]#0 move %[[VAL_25]] : (!fir.ref<!fir.char<1,3>>, i1) -> !hlfir.expr<!fir.char<1,3>>
! CHECK: hlfir.yield_element %[[VAL_26]] : !hlfir.expr<!fir.char<1,3>>

subroutine test7(x)
use types
integer :: x(:)
class(*), allocatable :: p(:)
p = inner(x)
contains
elemental function inner(x)
integer, intent(in) :: x
type(t1) :: inner
end function inner
end subroutine test7
! CHECK-LABEL: func.func @_QPtest7(
! CHECK: %[[VAL_12:.*]] = hlfir.elemental
! CHECK: %[[VAL_16:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = ".tmp.func_result"} : (!fir.ref<!fir.type<_QMtypesTt1>>) -> (!fir.ref<!fir.type<_QMtypesTt1>>, !fir.ref<!fir.type<_QMtypesTt1>>)
! CHECK: %[[VAL_17:.*]] = arith.constant false
! CHECK: %[[VAL_18:.*]] = hlfir.as_expr %[[VAL_16]]#0 move %[[VAL_17]] : (!fir.ref<!fir.type<_QMtypesTt1>>, i1) -> !hlfir.expr<!fir.type<_QMtypesTt1>>
! CHECK: hlfir.yield_element %[[VAL_18]] : !hlfir.expr<!fir.type<_QMtypesTt1>>

subroutine test8
if (associated(inner())) STOP 1
contains
function inner()
real, pointer :: inner
end function inner
end subroutine test8
! CHECK-LABEL: func.func @_QPtest8() {
! CHECK: %[[VAL_2:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = ".tmp.func_result"} : (!fir.ref<!fir.box<!fir.ptr<f32>>>) -> (!fir.ref<!fir.box<!fir.ptr<f32>>>, !fir.ref<!fir.box<!fir.ptr<f32>>>)
! CHECK: %[[VAL_3:.*]] = fir.load %[[VAL_2]]#1 : !fir.ref<!fir.box<!fir.ptr<f32>>>
! CHECK: %[[VAL_4:.*]] = fir.box_addr %[[VAL_3]] : (!fir.box<!fir.ptr<f32>>) -> !fir.ptr<f32>
12 changes: 8 additions & 4 deletions flang/test/Lower/HLFIR/where.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,21 @@ subroutine where_cleanup()
! CHECK: %[[VAL_6:.*]] = fir.call @_QPreturn_temporary_mask() fastmath<contract> : () -> !fir.box<!fir.heap<!fir.array<?x!fir.logical<4>>>>
! CHECK: fir.save_result %[[VAL_6]] to %[[VAL_1]] : !fir.box<!fir.heap<!fir.array<?x!fir.logical<4>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.logical<4>>>>>
! CHECK: %[[VAL_7:.*]]:2 = hlfir.declare %[[VAL_1]] {uniq_name = ".tmp.func_result"} : (!fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.logical<4>>>>>) -> (!fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.logical<4>>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.logical<4>>>>>)
! CHECK: %[[VAL_8:.*]] = fir.load %[[VAL_7]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.logical<4>>>>>
! CHECK: hlfir.yield %[[VAL_8]] : !fir.box<!fir.heap<!fir.array<?x!fir.logical<4>>>> cleanup {
! CHECK: %[[deref:.*]] = fir.load %[[VAL_7]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.logical<4>>>>>
! CHECK: %[[MustFree:.*]] = arith.constant false
! CHECK: %[[ResTemp:.*]] = hlfir.as_expr %[[deref]] move %[[MustFree]] : (!fir.box<!fir.heap<!fir.array<?x!fir.logical<4>>>>, i1) -> !hlfir.expr<?x!fir.logical<4>>
! CHECK: hlfir.yield %[[ResTemp]] : !hlfir.expr<?x!fir.logical<4>> cleanup {
! CHECK: fir.freemem
! CHECK: }
! CHECK: } do {
! CHECK: hlfir.region_assign {
! CHECK: %[[VAL_14:.*]] = fir.call @_QPreturn_temporary_array() fastmath<contract> : () -> !fir.box<!fir.heap<!fir.array<?xf32>>>
! CHECK: fir.save_result %[[VAL_14]] to %[[VAL_0]] : !fir.box<!fir.heap<!fir.array<?xf32>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xf32>>>>
! CHECK: %[[VAL_15:.*]]:2 = hlfir.declare %[[VAL_0]] {uniq_name = ".tmp.func_result"} : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf32>>>>) -> (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf32>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xf32>>>>)
! CHECK: %[[VAL_16:.*]] = fir.load %[[VAL_15]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf32>>>>
! CHECK: hlfir.yield %[[VAL_16]] : !fir.box<!fir.heap<!fir.array<?xf32>>> cleanup {
! CHECK: %[[deref:.*]] = fir.load %[[VAL_15]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf32>>>>
! CHECK: %[[MustFree:.*]] = arith.constant false
! CHECK: %[[ResTemp:.*]] = hlfir.as_expr %[[deref]] move %[[MustFree]] : (!fir.box<!fir.heap<!fir.array<?xf32>>>, i1) -> !hlfir.expr<?xf32>
! CHECK: hlfir.yield %[[ResTemp]] : !hlfir.expr<?xf32> cleanup {
! CHECK: fir.freemem
! CHECK: }
! CHECK: } to {
Expand Down
5 changes: 4 additions & 1 deletion flang/test/Lower/Intrinsics/storage_size-2.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ function return_char(l)
print*, storage_size(return_char(n))
! CHECK: %[[val_16:.*]] = fir.call @_QPreturn_char(%[[res_addr:[^,]*]], %[[res_len:[^,]*]], {{.*}})
! CHECK: %[[res:.*]]:2 = hlfir.declare %[[res_addr]] typeparams %[[res_len]]
! CHECK: %[[val_18:.*]] = fir.embox %[[res]]#1 typeparams %[[res_len]] : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>>
! CHECK: %[[false:.*]] = arith.constant false
! CHECK: %[[expr:.*]] = hlfir.as_expr %[[res]]#0 move %[[false]] : (!fir.boxchar<1>, i1) -> !hlfir.expr<!fir.char<1,?>>
! CHECK: %[[assoc:.*]]:3 = hlfir.associate %[[expr]] typeparams %[[res_len]] {uniq_name = "adapt.valuebyref"} : (!hlfir.expr<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>, i1)
! CHECK: %[[val_18:.*]] = fir.embox %[[assoc]]#1 typeparams %[[res_len]] : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>>
! CHECK: %[[val_19:.*]] = fir.box_elesize %[[val_18]] : (!fir.box<!fir.char<1,?>>) -> i32
! CHECK: %[[val_20:.*]] = arith.constant 8 : i32
! CHECK: %[[val_21:.*]] = arith.muli %[[val_19]], %[[val_20]] : i32
Expand Down