Skip to content

Commit

Permalink
[flang] load SECOND result in genSecond (#99342)
Browse files Browse the repository at this point in the history
Summary:
Until genSecond, all intrinsic `genXXX` returning scalar intrinsic
(except NULL) were returning them as value.

The code calling genIntrinsicCall is using that assumption when
generation the asExprOp because hflir.expr<> of scalar are badly
supported in tools (I should likely just forbid them all together), the
type is meant for "non trivial" values: arrays, character, and derived
type. For instance, the added tests crashed with error: `'arith.subf' op
operand #0 must be floating-point-like, but got '!hlfir.expr<f32>'`

Load the result in genSecond and add an assert after genIntrinsicCall to
better enforce this.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251583
  • Loading branch information
jeanPerier authored and yuxuanchen1997 committed Jul 25, 2024
1 parent 3d5d80e commit befc958
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 6 deletions.
2 changes: 2 additions & 0 deletions flang/lib/Lower/ConvertCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2005,6 +2005,8 @@ genIntrinsicRefCore(Fortran::lower::PreparedActualArguments &loweredActuals,
// returns a null pointer variable that should not be transformed into a value
// (what matters is the memory address).
if (resultEntity.isVariable() && intrinsicName != "null") {
assert(!fir::isa_trivial(fir::unwrapRefType(resultEntity.getType())) &&
"expect intrinsic scalar results to not be in memory");
hlfir::AsExprOp asExpr;
// Character/Derived MERGE lowering returns one of its argument address
// (this is the only intrinsic implemented in that way so far). The
Expand Down
2 changes: 1 addition & 1 deletion flang/lib/Optimizer/Builder/IntrinsicCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6161,7 +6161,7 @@ IntrinsicLibrary::genSecond(std::optional<mlir::Type> resultType,
genCpuTime(subroutineArgs);

if (resultType)
return result;
return builder.create<fir::LoadOp>(loc, fir::getBase(result));
return {};
}

Expand Down
28 changes: 23 additions & 5 deletions flang/test/Lower/Intrinsics/second.f90
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,28 @@ subroutine test_function(time)
! CHECK: %[[VAL_4:.*]] = fir.call @_FortranACpuTime() fastmath<contract> : () -> f64
! CHECK: %[[VAL_5:.*]] = fir.convert %[[VAL_4]] : (f64) -> f32
! CHECK: fir.store %[[VAL_5]] to %[[VAL_1]] : !fir.ref<f32>
! CHECK: %[[VAL_6:.*]]:2 = hlfir.declare %[[VAL_1]] {uniq_name = ".tmp.intrinsic_result"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
! CHECK: %[[VAL_7:.*]] = arith.constant false
! CHECK: %[[VAL_8:.*]] = hlfir.as_expr %[[VAL_6]]#0 move %[[VAL_7]] : (!fir.ref<f32>, i1) -> !hlfir.expr<f32>
! CHECK: hlfir.assign %[[VAL_8]] to %[[VAL_3]]#0 : !hlfir.expr<f32>, !fir.ref<f32>
! CHECK: hlfir.destroy %[[VAL_8]] : !hlfir.expr<f32>
! CHECK: %[[VAL_6:.*]] = fir.load %[[VAL_1]] : !fir.ref<f32>
! CHECK: hlfir.assign %[[VAL_6]] to %[[VAL_3]]#0 : f32, !fir.ref<f32>
! CHECK: return
! CHECK: }

subroutine test_function_subexpr(t1, t2)
real :: t1, t2
t2 = second() - t1
end subroutine
! CHECK-LABEL: func.func @_QPtest_function_subexpr(
! CHECK-SAME: %[[VAL_0:.*]]: !fir.ref<f32> {fir.bindc_name = "t1"},
! CHECK-SAME: %[[VAL_1:.*]]: !fir.ref<f32> {fir.bindc_name = "t2"}) {
! CHECK: %[[VAL_2:.*]] = fir.alloca f32
! CHECK: %[[VAL_3:.*]] = fir.dummy_scope : !fir.dscope
! CHECK: %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_0]] dummy_scope %[[VAL_3]] {uniq_name = "_QFtest_function_subexprEt1"} : (!fir.ref<f32>, !fir.dscope) -> (!fir.ref<f32>, !fir.ref<f32>)
! CHECK: %[[VAL_5:.*]]:2 = hlfir.declare %[[VAL_1]] dummy_scope %[[VAL_3]] {uniq_name = "_QFtest_function_subexprEt2"} : (!fir.ref<f32>, !fir.dscope) -> (!fir.ref<f32>, !fir.ref<f32>)
! CHECK: %[[VAL_6:.*]] = fir.call @_FortranACpuTime() fastmath<contract> : () -> f64
! CHECK: %[[VAL_7:.*]] = fir.convert %[[VAL_6]] : (f64) -> f32
! CHECK: fir.store %[[VAL_7]] to %[[VAL_2]] : !fir.ref<f32>
! CHECK: %[[VAL_8:.*]] = fir.load %[[VAL_2]] : !fir.ref<f32>
! CHECK: %[[VAL_9:.*]] = fir.load %[[VAL_4]]#0 : !fir.ref<f32>
! CHECK: %[[VAL_10:.*]] = arith.subf %[[VAL_8]], %[[VAL_9]] fastmath<contract> : f32
! CHECK: hlfir.assign %[[VAL_10]] to %[[VAL_5]]#0 : f32, !fir.ref<f32>
! CHECK: return
! CHECK: }

0 comments on commit befc958

Please sign in to comment.