[flang][hlfir] Fixed passing c_ptr arguments by value.

[vzakhari]

Summary

c_ptr arguments passed by value need special handling, which was missing
in HLFIR lowering. The lowering has to load the __address component
and pass the loaded value as the actual argument.

[jeanPerier]

Thanks for working on this TODO!

flang/lib/Lower/ConvertCall.cpp

1084

You cannot assume you will get an hlfir.expr here. This is the case in the example because this a a function result, but I think that with this modified example, a fir.ref<fir.type<cptr....>> would reach this spot:

subroutine test(cptr)
  use iso_c_binding
  interface
     subroutine get_expected_f(src) bind(c)
       use iso_c_binding
       type(c_ptr), value :: src
     end subroutine get_expected_f
  end interface
  type(c_ptr) :: cptr
  call get_expected_f(cptr)
end

Derived type are not part of the "trivial" definition of fir (fir::isa_trivial), so loadTrivialScalar did not turn it into an hlfir.expr.

So you will have to skip the associate/end_associate when value.isValue() is false.

[vzakhari]

flang/lib/Lower/ConvertCall.cpp

1084

Thank you for the example, Jean! I will upload the updated change shortly.

[jeanPerier]

Thanks, LGTM!

flang/lib/Lower/ConvertCall.cpp

if (type.isa<fir::CharacterType>()) {
     return fir::CharBoxValue(base, lengths[0]);
   }
   return base;
 }
 
 /// Lower a type(C_PTR/C_FUNPTR) argument with VALUE attribute into a
 /// reference. A C pointer can correspond to a Fortran dummy argument of type
 /// C_PTR with the VALUE attribute. (see 18.3.6 note 3).
-static mlir::Value
-genRecordCPtrValueArg(Fortran::lower::AbstractConverter &converter,
-                      mlir::Value rec, mlir::Type ty) {
-  fir::FirOpBuilder &builder = converter.getFirOpBuilder();
-  mlir::Location loc = converter.getCurrentLocation();
+static mlir::Value genRecordCPtrValueArg(fir::FirOpBuilder &builder,
+                                         mlir::Location loc, mlir::Value rec,
+                                         mlir::Type ty) {
   mlir::Value cAddr = fir::factory::genCPtrOrCFunptrAddr(builder, loc, rec, ty);
   mlir::Value cVal = builder.create<fir::LoadOp>(loc, cAddr);
   return builder.createConvert(loc, cAddr.getType(), cVal);
 }
 
 // Find the argument that corresponds to the host associations.
 // Verify some assumptions about how the signature was built here.
 [[maybe_unused]] static unsigned findHostAssocTuplePos(mlir::func::FuncOp fn) {
if (snd.isa<fir::BoxProcType>() &&
             loc, boxProcTy, llvm::ArrayRef<mlir::Value>{fst, host});
       } else {
         cast = builder.create<fir::EmboxProcOp>(loc, boxProcTy, fst);
       }
     } else {
       mlir::Type fromTy = fir::unwrapRefType(fst.getType());
       if (fir::isa_builtin_cptr_type(fromTy) &&
           Fortran::lower::isCPtrArgByValueType(snd)) {
-        cast = genRecordCPtrValueArg(converter, fst, fromTy);
+        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.
         TODO(loc, "derived type argument passed by value");
       } else {
         cast = builder.convertWithSemantics(loc, snd, fst,
                                             callingImplicitInterface);
       }
for (auto [preparedActual, arg] :
     if (!expr)
       TODO(loc, "assumed type actual argument");
 
     switch (arg.passBy) {
     case PassBy::Value: {
       // True pass-by-value semantics.
       assert(!preparedActual->handleDynamicOptional() && "cannot be optional");
       hlfir::Entity actual = preparedActual->getActual(loc, builder);
-      auto value = hlfir::loadTrivialScalar(loc, builder, actual);
-      if (!value.isValue())
-        TODO(loc, "Passing CPTR an CFUNCTPTR VALUE in HLFIR");
+      hlfir::Entity value = hlfir::loadTrivialScalar(loc, builder, actual);
+
+      mlir::Type eleTy = value.getFortranElementType();
+      if (fir::isa_builtin_cptr_type(eleTy)) {
+        // Pass-by-value argument of type(C_PTR/C_FUNPTR).
+        // Load the __address component and pass it by value.
+        if (value.isValue()) {
+          auto associate = hlfir::genAssociateExpr(loc, builder, value, eleTy,
+                                                   "adapt.cptrbyval");
+          value = hlfir::Entity{genRecordCPtrValueArg(
+              builder, loc, associate.getFirBase(), eleTy)};
+          builder.create<hlfir::EndAssociateOp>(loc, associate);
+        } else {
+          value =
+              hlfir::Entity{genRecordCPtrValueArg(builder, loc, value, eleTy)};
+        }
+      }
+
       caller.placeInput(arg, builder.createConvert(loc, argTy, value));
     } break;
     case PassBy::BaseAddressValueAttribute:
     case PassBy::CharBoxValueAttribute:
     case PassBy::Box:
     case PassBy::BaseAddress:
     case PassBy::BoxChar: {
       PreparedDummyArgument preparedDummy = prepareUserCallActualArgument(

flang/test/HLFIR/c_ptr_byvalue.f90

+! RUN: bbc -emit-fir -hlfir %s -o - | FileCheck %s
+
+! CHECK-LABEL:   func.func @_QPtest1() {
+! CHECK:           %[[VAL_110:.*]]:3 = hlfir.associate %{{.*}} {uniq_name = "adapt.cptrbyval"} : (!hlfir.expr<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>) -> (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, i1)
+! CHECK:           %[[VAL_111:.*]] = fir.field_index __address, !fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>
+! CHECK:           %[[VAL_112:.*]] = fir.coordinate_of %[[VAL_110]]#1, %[[VAL_111]] : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, !fir.field) -> !fir.ref<i64>
+! CHECK:           %[[VAL_113:.*]] = fir.load %[[VAL_112]] : !fir.ref<i64>
+! CHECK:           %[[VAL_114:.*]] = fir.convert %[[VAL_113]] : (i64) -> !fir.ref<i64>
+! CHECK:           hlfir.end_associate %[[VAL_110]]#1, %[[VAL_110]]#2 : !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, i1
+! CHECK:           fir.call @get_expected_f(%[[VAL_114]]) fastmath<contract> : (!fir.ref<i64>) -> ()
+subroutine test1
+  use iso_c_binding
+  interface
+     subroutine get_expected_f(src) bind(c)
+       use iso_c_binding
+       type(c_ptr), value :: src
+     end subroutine get_expected_f
+  end interface
+  real, target,  dimension(1) :: r_src
+  call get_expected_f(c_loc(r_src))
+end
+
+! CHECK-LABEL:   func.func @_QPtest2(
+! CHECK-SAME:                        %[[VAL_0:.*]]: !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>> {fir.bindc_name = "cptr"}) {
+! CHECK:           %[[VAL_97:.*]]:2 = hlfir.declare %[[VAL_0]] {uniq_name = "_QFtest2Ecptr"} : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>) -> (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>)
+! CHECK:           %[[VAL_98:.*]] = fir.field_index __address, !fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>
+! CHECK:           %[[VAL_99:.*]] = fir.coordinate_of %[[VAL_97]]#0, %[[VAL_98]] : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, !fir.field) -> !fir.ref<i64>
+! CHECK:           %[[VAL_100:.*]] = fir.load %[[VAL_99]] : !fir.ref<i64>
+! CHECK:           %[[VAL_101:.*]] = fir.convert %[[VAL_100]] : (i64) -> !fir.ref<i64>
+! CHECK:           fir.call @get_expected_f(%[[VAL_101]]) fastmath<contract> : (!fir.ref<i64>) -> ()
+subroutine test2(cptr)
+  use iso_c_binding
+  interface
+     subroutine get_expected_f(src) bind(c)
+       use iso_c_binding
+       type(c_ptr), value :: src
+     end subroutine get_expected_f
+  end interface
+  type(c_ptr) :: cptr
+  call get_expected_f(cptr)
+end