-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[flang][hlfir] patch for assumed shape dummy with VALUE keyword when lowering to HLFIR #70391
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.
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
Conversation
@llvm/pr-subscribers-flang-fir-hlfir Author: Anthony Cabrera (cabreraam) ChangesAdds functionality for assumed shape dummy with value keyword Full diff: https://github.com/llvm/llvm-project/pull/70391.diff 2 Files Affected:
diff --git a/flang/lib/Lower/CallInterface.cpp b/flang/lib/Lower/CallInterface.cpp
index 379d9be0e53a3dc..2ebc2f5b6612103 100644
--- a/flang/lib/Lower/CallInterface.cpp
+++ b/flang/lib/Lower/CallInterface.cpp
@@ -964,10 +964,14 @@ class Fortran::lower::CallInterfaceImpl {
addPassedArg(PassEntityBy::MutableBox, entity, characteristics);
} else if (dummyRequiresBox(obj, isBindC)) {
// Pass as fir.box or fir.class
- if (isValueAttr)
- TODO(loc, "assumed shape dummy argument with VALUE attribute");
- addFirOperand(boxType, nextPassedArgPosition(), Property::Box, attrs);
- addPassedArg(PassEntityBy::Box, entity, characteristics);
+ Property prop = Property::Box;
+ PassEntityBy passBy = PassEntityBy::Box;
+ if (isValueAttr) {
+ passBy = PassEntityBy::BaseAddressValueAttribute;
+ prop = Property::Value;
+ }
+ addFirOperand(boxType, nextPassedArgPosition(), prop, attrs);
+ addPassedArg(passBy, entity, characteristics);
} else if (dynamicType.category() ==
Fortran::common::TypeCategory::Character) {
// Pass as fir.box_char
diff --git a/flang/test/HLFIR/assumed_shape_with_value_keyword.f90 b/flang/test/HLFIR/assumed_shape_with_value_keyword.f90
new file mode 100644
index 000000000000000..b5080d9bedca690
--- /dev/null
+++ b/flang/test/HLFIR/assumed_shape_with_value_keyword.f90
@@ -0,0 +1,149 @@
+! RUN: bbc -emit-hlfir %s -o - | FileCheck %s
+
+! Addresses assumed shape dummy argument with VALUE keyword
+
+subroutine test_integer_value1(x)
+ integer, value :: x(:)
+ call internal_call1(x)
+end
+
+! CHECK-LABEL: func.func @_QPtest_integer_value1(
+! CHECK-SAME: %[[ARG0:.*]]: !fir.box<!fir.array<?xi32>> {fir.bindc_name = "x"}) {
+! CHECK: %[[VAL_0:.*]]:2 = hlfir.declare %[[ARG0]] {fortran_attrs = #fir.var_attrs<value>, uniq_name = "_QFtest_integer_value1Ex"} : (!fir.box<!fir.array<?xi32>>) -> (!fir.box<!fir.array<?xi32>>, !fir.box<!fir.array<?xi32>>)
+! CHECK: %[[VAL_1:.*]]:2 = hlfir.copy_in %[[VAL_0]]#0 : (!fir.box<!fir.array<?xi32>>) -> (!fir.box<!fir.array<?xi32>>, i1)
+! CHECK: %[[VAL_2:.*]] = fir.box_addr %[[VAL_1]]#0 : (!fir.box<!fir.array<?xi32>>) -> !fir.ref<!fir.array<?xi32>>
+! CHECK: fir.call @_QPinternal_call1(%[[VAL_2]]) fastmath<contract> : (!fir.ref<!fir.array<?xi32>>) -> ()
+! CHECK: hlfir.copy_out %[[VAL_1]]#0, %[[VAL_1]]#1 to %[[VAL_0]]#0 : (!fir.box<!fir.array<?xi32>>, i1, !fir.box<!fir.array<?xi32>>) -> ()
+! CHECK: return
+! CHECK: }
+
+subroutine test_integer_value2(x)
+ integer, value :: x(:,:)
+ call internal_call2(x)
+end
+! CHECK-LABEL: func.func @_QPtest_integer_value2(
+! CHECK-SAME: %[[ARG0:.*]]: !fir.box<!fir.array<?x?xi32>> {fir.bindc_name = "x"}) {
+! CHECK: %[[VAL_0:.*]]:2 = hlfir.declare %[[ARG0]] {fortran_attrs = #fir.var_attrs<value>, uniq_name = "_QFtest_integer_value2Ex"} : (!fir.box<!fir.array<?x?xi32>>) -> (!fir.box<!fir.array<?x?xi32>>, !fir.box<!fir.array<?x?xi32>>)
+! CHECK: %[[VAL_1:.*]]:2 = hlfir.copy_in %[[VAL_0]]#0 : (!fir.box<!fir.array<?x?xi32>>) -> (!fir.box<!fir.array<?x?xi32>>, i1)
+! CHECK: %[[VAL_2:.*]] = fir.box_addr %[[VAL_1]]#0 : (!fir.box<!fir.array<?x?xi32>>) -> !fir.ref<!fir.array<?x?xi32>>
+! CHECK: fir.call @_QPinternal_call2(%[[VAL_2]]) fastmath<contract> : (!fir.ref<!fir.array<?x?xi32>>) -> ()
+! CHECK: hlfir.copy_out %[[VAL_1]]#0, %[[VAL_1]]#1 to %[[VAL_0]]#0 : (!fir.box<!fir.array<?x?xi32>>, i1, !fir.box<!fir.array<?x?xi32>>) -> ()
+! CHECK: return
+! CHECK: }
+
+subroutine test_real_value1(x)
+ real, value :: x(:)
+ call internal_call3(x)
+end
+! CHECK-LABEL: func.func @_QPtest_real_value1(
+! CHECK-SAME: %[[ARG0:.*]]: !fir.box<!fir.array<?xf32>> {fir.bindc_name = "x"}) {
+! CHECK: %[[VAL_0:.*]]:2 = hlfir.declare %[[ARG0]] {fortran_attrs = #fir.var_attrs<value>, uniq_name = "_QFtest_real_value1Ex"} : (!fir.box<!fir.array<?xf32>>) -> (!fir.box<!fir.array<?xf32>>, !fir.box<!fir.array<?xf32>>)
+! CHECK: %[[VAL_1:.*]]:2 = hlfir.copy_in %[[VAL_0]]#0 : (!fir.box<!fir.array<?xf32>>) -> (!fir.box<!fir.array<?xf32>>, i1)
+! CHECK: %[[VAL_2:.*]] = fir.box_addr %[[VAL_1]]#0 : (!fir.box<!fir.array<?xf32>>) -> !fir.ref<!fir.array<?xf32>>
+! CHECK: fir.call @_QPinternal_call3(%[[VAL_2]]) fastmath<contract> : (!fir.ref<!fir.array<?xf32>>) -> ()
+! CHECK: hlfir.copy_out %[[VAL_1]]#0, %[[VAL_1]]#1 to %[[VAL_0]]#0 : (!fir.box<!fir.array<?xf32>>, i1, !fir.box<!fir.array<?xf32>>) -> ()
+! CHECK: return
+! CHECK: }
+
+subroutine test_real_value2(x)
+ real, value :: x(:,:)
+ call internal_call4(x)
+end
+! CHECK-LABEL: func.func @_QPtest_real_value2(
+! CHECK-SAME: %[[ARG0:.*]]: !fir.box<!fir.array<?x?xf32>> {fir.bindc_name = "x"}) {
+! CHECK: %[[VAL_0:.*]]:2 = hlfir.declare %[[ARG0]] {fortran_attrs = #fir.var_attrs<value>, uniq_name = "_QFtest_real_value2Ex"} : (!fir.box<!fir.array<?x?xf32>>) -> (!fir.box<!fir.array<?x?xf32>>, !fir.box<!fir.array<?x?xf32>>)
+! CHECK: %[[VAL_1:.*]]:2 = hlfir.copy_in %[[VAL_0]]#0 : (!fir.box<!fir.array<?x?xf32>>) -> (!fir.box<!fir.array<?x?xf32>>, i1)
+! CHECK: %[[VAL_2:.*]] = fir.box_addr %[[VAL_1]]#0 : (!fir.box<!fir.array<?x?xf32>>) -> !fir.ref<!fir.array<?x?xf32>>
+! CHECK: fir.call @_QPinternal_call4(%[[VAL_2]]) fastmath<contract> : (!fir.ref<!fir.array<?x?xf32>>) -> ()
+! CHECK: hlfir.copy_out %[[VAL_1]]#0, %[[VAL_1]]#1 to %[[VAL_0]]#0 : (!fir.box<!fir.array<?x?xf32>>, i1, !fir.box<!fir.array<?x?xf32>>) -> ()
+! CHECK: return
+! CHECK: }
+
+subroutine test_complex_value1(x)
+ complex, value :: x(:)
+ call internal_call5(x)
+end
+! CHECK-LABEL: func.func @_QPtest_complex_value1(
+! CHECK-SAME: %[[ARG0:.*]]: !fir.box<!fir.array<?x!fir.complex<4>>> {fir.bindc_name = "x"}) {
+! CHECK: %[[VAL_0:.*]]:2 = hlfir.declare %[[ARG0]] {fortran_attrs = #fir.var_attrs<value>, uniq_name = "_QFtest_complex_value1Ex"} : (!fir.box<!fir.array<?x!fir.complex<4>>>) -> (!fir.box<!fir.array<?x!fir.complex<4>>>, !fir.box<!fir.array<?x!fir.complex<4>>>)
+! CHECK: %[[VAL_1:.*]]:2 = hlfir.copy_in %[[VAL_0]]#0 : (!fir.box<!fir.array<?x!fir.complex<4>>>) -> (!fir.box<!fir.array<?x!fir.complex<4>>>, i1)
+! CHECK: %[[VAL_2:.*]] = fir.box_addr %[[VAL_1]]#0 : (!fir.box<!fir.array<?x!fir.complex<4>>>) -> !fir.ref<!fir.array<?x!fir.complex<4>>>
+! CHECK: fir.call @_QPinternal_call5(%[[VAL_2]]) fastmath<contract> : (!fir.ref<!fir.array<?x!fir.complex<4>>>) -> ()
+! CHECK: hlfir.copy_out %[[VAL_1]]#0, %[[VAL_1]]#1 to %[[VAL_0]]#0 : (!fir.box<!fir.array<?x!fir.complex<4>>>, i1, !fir.box<!fir.array<?x!fir.complex<4>>>) -> ()
+! CHECK: return
+! CHECK: }
+
+subroutine test_complex_value2(x)
+ complex, value :: x(:,:)
+ call internal_call6(x)
+end
+! CHECK-LABEL: func.func @_QPtest_complex_value2(
+! CHECK-SAME: %[[ARG0:.*]]: !fir.box<!fir.array<?x?x!fir.complex<4>>> {fir.bindc_name = "x"}) {
+! CHECK: %[[VAL_0:.*]]:2 = hlfir.declare %[[ARG0]] {fortran_attrs = #fir.var_attrs<value>, uniq_name = "_QFtest_complex_value2Ex"} : (!fir.box<!fir.array<?x?x!fir.complex<4>>>) -> (!fir.box<!fir.array<?x?x!fir.complex<4>>>, !fir.box<!fir.array<?x?x!fir.complex<4>>>)
+! CHECK: %[[VAL_1:.*]]:2 = hlfir.copy_in %[[VAL_0]]#0 : (!fir.box<!fir.array<?x?x!fir.complex<4>>>) -> (!fir.box<!fir.array<?x?x!fir.complex<4>>>, i1)
+! CHECK: %[[VAL_2:.*]] = fir.box_addr %[[VAL_1]]#0 : (!fir.box<!fir.array<?x?x!fir.complex<4>>>) -> !fir.ref<!fir.array<?x?x!fir.complex<4>>>
+! CHECK: fir.call @_QPinternal_call6(%[[VAL_2]]) fastmath<contract> : (!fir.ref<!fir.array<?x?x!fir.complex<4>>>) -> ()
+! CHECK: hlfir.copy_out %[[VAL_1]]#0, %[[VAL_1]]#1 to %[[VAL_0]]#0 : (!fir.box<!fir.array<?x?x!fir.complex<4>>>, i1, !fir.box<!fir.array<?x?x!fir.complex<4>>>) -> ()
+! CHECK: return
+! CHECK: }
+
+subroutine test_optional1(x)
+ real, value, optional :: x(:)
+ if (present(x)) then
+ call internal_call7(x)
+ endif
+end
+! CHECK-LABEL: func.func @_QPtest_optional1(
+! CHECK-SAME: %[[ARG0:.*]]: !fir.box<!fir.array<?xf32>> {fir.bindc_name = "x", fir.optional}) {
+! CHECK: %[[VAL_0:.*]]:2 = hlfir.declare %[[ARG0]] {fortran_attrs = #fir.var_attrs<optional, value>, uniq_name = "_QFtest_optional1Ex"} : (!fir.box<!fir.array<?xf32>>) -> (!fir.box<!fir.array<?xf32>>, !fir.box<!fir.array<?xf32>>)
+! CHECK: %[[VAL_1:.*]] = fir.is_present %[[VAL_0]]#1 : (!fir.box<!fir.array<?xf32>>) -> i1
+! CHECK: fir.if %[[VAL_1:.*]] {
+! CHECK: %[[VAL_2:.*]]:2 = hlfir.copy_in %[[VAL_0]]#0 : (!fir.box<!fir.array<?xf32>>) -> (!fir.box<!fir.array<?xf32>>, i1)
+! CHECK: %[[VAL_3:.*]] = fir.box_addr %[[VAL_2]]#0 : (!fir.box<!fir.array<?xf32>>) -> !fir.ref<!fir.array<?xf32>>
+! CHECK: fir.call @_QPinternal_call7(%[[VAL_3]]) fastmath<contract> : (!fir.ref<!fir.array<?xf32>>) -> ()
+! CHECK: hlfir.copy_out %[[VAL_2]]#0, %[[VAL_2]]#1 to %[[VAL_0]]#0 : (!fir.box<!fir.array<?xf32>>, i1, !fir.box<!fir.array<?xf32>>) -> ()
+! CHECK: } else {
+! CHECK: }
+! CHECK: return
+! CHECK: }
+
+subroutine test_optional2(x)
+ real, value, optional :: x(:,:)
+ if (present(x)) then
+ call internal_call8(x)
+ endif
+end
+! CHECK-LABEL: func.func @_QPtest_optional2(
+! CHECK-SAME: %[[ARG0:.*]]: !fir.box<!fir.array<?x?xf32>> {fir.bindc_name = "x", fir.optional}) {
+! CHECK: %[[VAL_0:.*]]:2 = hlfir.declare %[[ARG0]] {fortran_attrs = #fir.var_attrs<optional, value>, uniq_name = "_QFtest_optional2Ex"} : (!fir.box<!fir.array<?x?xf32>>) -> (!fir.box<!fir.array<?x?xf32>>, !fir.box<!fir.array<?x?xf32>>)
+! CHECK: %[[VAL_1:.*]] = fir.is_present %[[VAL_0]]#1 : (!fir.box<!fir.array<?x?xf32>>) -> i1
+! CHECK: fir.if %[[VAL_1:.*]] {
+! CHECK: %[[VAL_2:.*]]:2 = hlfir.copy_in %[[VAL_0]]#0 : (!fir.box<!fir.array<?x?xf32>>) -> (!fir.box<!fir.array<?x?xf32>>, i1)
+! CHECK: %[[VAL_3:.*]] = fir.box_addr %[[VAL_2]]#0 : (!fir.box<!fir.array<?x?xf32>>) -> !fir.ref<!fir.array<?x?xf32>>
+! CHECK: fir.call @_QPinternal_call8(%[[VAL_3]]) fastmath<contract> : (!fir.ref<!fir.array<?x?xf32>>) -> ()
+! CHECK: hlfir.copy_out %[[VAL_2]]#0, %[[VAL_2]]#1 to %[[VAL_0]]#0 : (!fir.box<!fir.array<?x?xf32>>, i1, !fir.box<!fir.array<?x?xf32>>) -> ()
+! CHECK: } else {
+! CHECK: }
+! CHECK: return
+! CHECK: }
+
+subroutine test_optional3(x)
+ real, value, optional :: x(:)
+ if (present(x)) then
+ stop
+ endif
+end
+! CHECK-LABEL: func.func @_QPtest_optional3(
+! CHECK-SAME: %[[ARG0:.*]]: !fir.box<!fir.array<?xf32>> {fir.bindc_name = "x", fir.optional}) {
+! CHECK: %[[VAL_0:.*]]:2 = hlfir.declare %[[ARG0]] {fortran_attrs = #fir.var_attrs<optional, value>, uniq_name = "_QFtest_optional3Ex"} : (!fir.box<!fir.array<?xf32>>) -> (!fir.box<!fir.array<?xf32>>, !fir.box<!fir.array<?xf32>>)
+! CHECK: %[[VAL_1:.*]] = fir.is_present %[[VAL_0]]#1 : (!fir.box<!fir.array<?xf32>>) -> i1
+! CHECK: cf.cond_br %[[VAL_1]], ^bb1, ^bb2
+! CHECK: b1: // pred: ^bb0
+! CHECK: %[[C0_I32:.*]] = arith.constant 0 : i32
+! CHECK: %[[FALSE:.*]] = arith.constant false
+! CHECK: %[[FALSE_0:.*]] = arith.constant false
+! CHECK: %[[VAL_2:.*]] = fir.call @_FortranAStopStatement(%[[C0_I32]], %[[FALSE]], %[[FALSE]]_0) fastmath<contract> : (i32, i1, i1) -> none
+! CHECK: fir.unreachable
+! CHECK: b2: // pred: ^bb0
+! CHECK: return
+! CHECK: }
\ No newline at end of file
|
3f7dbea
to
fe845c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this.
flang/lib/Lower/CallInterface.cpp
Outdated
Property prop = Property::Box; | ||
PassEntityBy passBy = PassEntityBy::Box; | ||
if (isValueAttr) { | ||
passBy = PassEntityBy::BaseAddressValueAttribute; | ||
prop = Property::Value; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PassEntityBy::BaseAddressValueAttribute and Property::Value are incorrect here (they first implies the argument should be prepared as a raw address for a VALUE, and the second says that the FIR argument at that position is a value in register).
This makes no difference on the callee side, but if you add tests that calls procedure with such arguments, there will likely be issues since BaseAddressValueAttribute implies the base address should be passed.
I think you will want to just keep it as it is, the VALUE attribute is checked in HLFIR here
llvm-project/flang/lib/Lower/ConvertCall.cpp
Line 917 in e4dc7d4
if (arg.hasValueAttribute() || |
Without HLFIR, there is a need for a new BoxValueAttribute category and special handling for it (that is why I think the TODO should only be lifted when lowering to HLFIR converter.getLoweringOptions().getLowerToHighLevelFIR()
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! I integrated this feedback into a new commit. Let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update, please remove the commented code (and I would avoid creating the "prop" and "passBy" variable and keep the original code instead). Otherwise LGTM.
fe845c7
to
67fcba2
Compare
67fcba2
to
74ba369
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM
Adds functionality for assumed shape dummy with value keyword when lowering to HLFIR