-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
[flang] Lower VALUE derived types in BIND(C) interface #74847
[flang] Lower VALUE derived types in BIND(C) interface #74847
Conversation
VALUE derived type are passed by reference outside of BIND(C) interface. The ABI is much simpler and it is possible for these arguments to have the OPTIONAL attribute. In the BIND(C) context, these arguments must follow the C ABI for struct, which may lead the data to be passed in register. OPTIONAL is also forbidden for those arguments, so it is safe to directly use the fir.type<T> type for the func.func argument. Codegen is in charge of later applying the C passing ABI according to the target (llvm#74829).
@llvm/pr-subscribers-flang-fir-hlfir Author: None (jeanPerier) ChangesVALUE derived type are passed by reference outside of BIND(C) interface. The ABI is much simpler and it is possible for these arguments to have the OPTIONAL attribute. In the BIND(C) context, these arguments must follow the C ABI for struct, which may lead the data to be passed in register. OPTIONAL is also forbidden for those arguments, so it is safe to directly use the fir.type<T> type for the func.func argument. Codegen is in charge of later applying the C passing ABI according to the target (#74829). Full diff: https://github.com/llvm/llvm-project/pull/74847.diff 3 Files Affected:
diff --git a/flang/lib/Lower/ConvertCall.cpp b/flang/lib/Lower/ConvertCall.cpp
index 81f4c0a2c6d2d..49e70181a668c 100644
--- a/flang/lib/Lower/ConvertCall.cpp
+++ b/flang/lib/Lower/ConvertCall.cpp
@@ -360,9 +360,8 @@ fir::ExtendedValue Fortran::lower::genCallOpAndResult(
if (fir::isa_builtin_cptr_type(fromTy) &&
Fortran::lower::isCPtrArgByValueType(snd)) {
cast = genRecordCPtrValueArg(builder, loc, fst, fromTy);
- } else if (fir::isa_derived(snd)) {
- // FIXME: This seems like a serious bug elsewhere in lowering. Paper
- // over the problem for now.
+ } else if (fir::isa_derived(snd) && !fir::isa_derived(fst.getType())) {
+ // TODO: remove this TODO once the old lowering is gone.
TODO(loc, "derived type argument passed by value");
} else {
cast = builder.convertWithSemantics(loc, snd, fst,
@@ -1188,8 +1187,18 @@ genUserCall(Fortran::lower::PreparedActualArguments &loweredActuals,
value =
hlfir::Entity{genRecordCPtrValueArg(builder, loc, value, eleTy)};
}
+ } else if (fir::isa_derived(value.getFortranElementType())) {
+ // BIND(C), VALUE derived type. The derived type value must really
+ // be loaded here.
+ auto [derived, cleanup] = hlfir::convertToValue(loc, builder, value);
+ mlir::Value loadedValue = fir::getBase(derived);
+ if (fir::isa_ref_type(loadedValue.getType()))
+ loadedValue = builder.create<fir::LoadOp>(loc, loadedValue);
+ caller.placeInput(arg, loadedValue);
+ if (cleanup)
+ (*cleanup)();
+ break;
}
-
caller.placeInput(arg, builder.createConvert(loc, argTy, value));
} break;
case PassBy::BaseAddressValueAttribute:
diff --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp
index 676fecdb52a8b..364de33d00a62 100644
--- a/flang/lib/Lower/ConvertVariable.cpp
+++ b/flang/lib/Lower/ConvertVariable.cpp
@@ -2138,8 +2138,6 @@ void Fortran::lower::mapSymbolAttributes(
if (isCptrByVal || !fir::conformsWithPassByRef(argType)) {
// Dummy argument passed in register. Place the value in memory at that
// point since lowering expect symbols to be mapped to memory addresses.
- if (argType.isa<fir::RecordType>())
- TODO(loc, "derived type argument passed by value");
mlir::Type symType = converter.genType(sym);
addr = builder.create<fir::AllocaOp>(loc, symType);
if (isCptrByVal) {
diff --git a/flang/test/Lower/HLFIR/bindc-value-derived.f90 b/flang/test/Lower/HLFIR/bindc-value-derived.f90
new file mode 100644
index 0000000000000..671e6d45b9a99
--- /dev/null
+++ b/flang/test/Lower/HLFIR/bindc-value-derived.f90
@@ -0,0 +1,37 @@
+! Test lowering of derived types passed with VALUE attribute in BIND(C)
+! interface. They are passed as fir.type<T> value. The actual C struct
+! passing ABI is done in code generation according to the target.
+
+! RUN: bbc -emit-hlfir -o - -I nw %s 2>&1 | FileCheck %s
+
+module bindc_byval
+ type, bind(c) :: t
+ integer :: i
+ end type
+contains
+ subroutine test(x) bind(c)
+ type(t), value :: x
+ call use_it(x%i)
+ end subroutine
+! CHECK-LABEL: func.func @test(
+! CHECK-SAME: %[[VAL_0:.*]]: !fir.type<_QMbindc_byvalTt{i:i32}> {fir.bindc_name = "x"}) attributes {fir.bindc_name = "test"} {
+! CHECK: %[[VAL_1:.*]] = fir.alloca !fir.type<_QMbindc_byvalTt{i:i32}>
+! CHECK: fir.store %[[VAL_0]] to %[[VAL_1]] : !fir.ref<!fir.type<_QMbindc_byvalTt{i:i32}>>
+! CHECK: %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_1]] {fortran_attrs = #fir.var_attrs<value>, uniq_name = "_QMbindc_byvalFtestEx"} : (!fir.ref<!fir.type<_QMbindc_byvalTt{i:i32}>>) -> (!fir.ref<!fir.type<_QMbindc_byvalTt{i:i32}>>, !fir.ref<!fir.type<_QMbindc_byvalTt{i:i32}>>)
+! CHECK: %[[VAL_3:.*]] = hlfir.designate %[[VAL_2]]#0{"i"} : (!fir.ref<!fir.type<_QMbindc_byvalTt{i:i32}>>) -> !fir.ref<i32>
+! CHECK: fir.call @_QPuse_it(%[[VAL_3]]) fastmath<contract> : (!fir.ref<i32>) -> ()
+! CHECK: return
+! CHECK: }
+
+ subroutine call_it(x)
+ type(t) x
+ call test(x)
+ end subroutine
+! CHECK-LABEL: func.func @_QMbindc_byvalPcall_it(
+! CHECK-SAME: %[[VAL_0:.*]]: !fir.ref<!fir.type<_QMbindc_byvalTt{i:i32}>> {fir.bindc_name = "x"}) {
+! CHECK: %[[VAL_1:.*]]:2 = hlfir.declare %[[VAL_0]] {uniq_name = "_QMbindc_byvalFcall_itEx"} : (!fir.ref<!fir.type<_QMbindc_byvalTt{i:i32}>>) -> (!fir.ref<!fir.type<_QMbindc_byvalTt{i:i32}>>, !fir.ref<!fir.type<_QMbindc_byvalTt{i:i32}>>)
+! CHECK: %[[VAL_2:.*]] = fir.load %[[VAL_1]]#1 : !fir.ref<!fir.type<_QMbindc_byvalTt{i:i32}>>
+! CHECK: fir.call @test(%[[VAL_2]]) fastmath<contract> : (!fir.type<_QMbindc_byvalTt{i:i32}>) -> ()
+! CHECK: return
+! CHECK: }
+end module
|
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.
Looks great!
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.
LGTM. Thanks!
VALUE derived type are passed by reference outside of BIND(C) interface. The ABI is much simpler and it is possible for these arguments to have the OPTIONAL attribute.
In the BIND(C) context, these arguments must follow the C ABI for struct, which may lead the data to be passed in register. OPTIONAL is also forbidden for those arguments, so it is safe to directly use the fir.type type for the func.func argument.
Codegen is in charge of later applying the C passing ABI according to the target (#74829).