[flang-commits] [flang] [flang] Lower VALUE derived types in BIND(C) interface (PR #74847)

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-fir-hlfir

Author: None (jeanPerier)

<details>
<summary>Changes</summary>

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 (https://github.com/llvm/llvm-project/pull/74829).

---
Full diff: https://github.com/llvm/llvm-project/pull/74847.diff


3 Files Affected:

- (modified) flang/lib/Lower/ConvertCall.cpp (+13-4) 
- (modified) flang/lib/Lower/ConvertVariable.cpp (-2) 
- (added) flang/test/Lower/HLFIR/bindc-value-derived.f90 (+37) 


``````````diff
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

``````````

</details>


https://github.com/llvm/llvm-project/pull/74847