From d7f5c644549df294e790a669154e00ccc1bdc2ef Mon Sep 17 00:00:00 2001 From: Carlos Seo Date: Fri, 5 Sep 2025 21:27:50 +0000 Subject: [PATCH 1/4] [Flang] Handle %VAL arguments correctly Internal procedures expect reference parameters. However, when %VAL is used, the argument should be passed by value. Create a temporary variable in call generation and pass the address to avoid a type conversion error. Fixes #118239 --- flang/lib/Lower/ConvertCall.cpp | 33 ++++++++++++++++--- .../Lower/percent-val-actual-argument.f90 | 14 ++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 flang/test/Lower/percent-val-actual-argument.f90 diff --git a/flang/lib/Lower/ConvertCall.cpp b/flang/lib/Lower/ConvertCall.cpp index 04dcc9250be61..9f71f7417005f 100644 --- a/flang/lib/Lower/ConvertCall.cpp +++ b/flang/lib/Lower/ConvertCall.cpp @@ -494,10 +494,20 @@ Fortran::lower::genCallOpAndResult( // arguments of any type and vice versa. mlir::Value cast; auto *context = builder.getContext(); - if (mlir::isa(snd) && - mlir::isa(fst.getType())) { - auto funcTy = mlir::FunctionType::get(context, {}, {}); - auto boxProcTy = builder.getBoxProcType(funcTy); + + // Special handling for %VAL arguments: internal procedures expect + // reference parameters. When %VAL is used, the argument should be + // passed by value. So we need to create a temporary variable and + // pass its address to avoid a type conversion error. + if (fir::isa_ref_type(snd) && !fir::isa_ref_type(fst.getType()) && + fir::dyn_cast_ptrEleTy(snd) == fst.getType()) { + mlir::Value temp = builder.createTemporary(loc, fst.getType()); + builder.create(loc, fst, temp); + cast = temp; + } else if (mlir::isa(snd) && + mlir::isa(fst.getType())) { + mlir::FunctionType funcTy = mlir::FunctionType::get(context, {}, {}); + fir::BoxProcType boxProcTy = builder.getBoxProcType(funcTy); if (mlir::Value host = argumentHostAssocs(converter, fst)) { cast = fir::EmboxProcOp::create(builder, loc, boxProcTy, llvm::ArrayRef{fst, host}); @@ -1637,7 +1647,20 @@ void prepareUserCallArguments( (*cleanup)(); break; } - caller.placeInput(arg, builder.createConvert(loc, argTy, value)); + // For %VAL arguments, we should pass the value directly without + // conversion to reference types. If argTy is different from value type, + // it might be due to signature mismatch with internal procedures. + if (argTy == value.getType()) + caller.placeInput(arg, value); + else if (fir::isa_ref_type(argTy) && + fir::dyn_cast_ptrEleTy(argTy) == value.getType()) { + // We're trying to convert value to reference - create temporary + mlir::Value temp = builder.createTemporary(loc, value.getType()); + builder.create(loc, value, temp); + caller.placeInput(arg, temp); + } else + caller.placeInput(arg, builder.createConvert(loc, argTy, value)); + } break; case PassBy::BaseAddressValueAttribute: case PassBy::CharBoxValueAttribute: diff --git a/flang/test/Lower/percent-val-actual-argument.f90 b/flang/test/Lower/percent-val-actual-argument.f90 new file mode 100644 index 0000000000000..736baa0eca21d --- /dev/null +++ b/flang/test/Lower/percent-val-actual-argument.f90 @@ -0,0 +1,14 @@ +! RUN: bbc %s -emit-fir -o - | FileCheck %s + +program main + logical::a1 + data a1/.true./ + call sa(%val(a1)) +! CHECK: fir.load %3 : !fir.ref> +! CHECK: fir.convert %13 : (!fir.logical<4>) -> i1 + write(6,*) "a1 = ", a1 +end program main + +subroutine sa(x1) + logical::x1 +end subroutine sa From 747ede1e609553a53f10bb349c0914d29c3a7beb Mon Sep 17 00:00:00 2001 From: Carlos Seo Date: Mon, 8 Sep 2025 14:49:24 +0000 Subject: [PATCH 2/4] Remove unnecessary temporary storage creation --- flang/lib/Lower/ConvertCall.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/flang/lib/Lower/ConvertCall.cpp b/flang/lib/Lower/ConvertCall.cpp index 9f71f7417005f..095f6295d6ad7 100644 --- a/flang/lib/Lower/ConvertCall.cpp +++ b/flang/lib/Lower/ConvertCall.cpp @@ -497,13 +497,12 @@ Fortran::lower::genCallOpAndResult( // Special handling for %VAL arguments: internal procedures expect // reference parameters. When %VAL is used, the argument should be - // passed by value. So we need to create a temporary variable and - // pass its address to avoid a type conversion error. + // passed by value. Pass the originally loaded value. if (fir::isa_ref_type(snd) && !fir::isa_ref_type(fst.getType()) && fir::dyn_cast_ptrEleTy(snd) == fst.getType()) { - mlir::Value temp = builder.createTemporary(loc, fst.getType()); - builder.create(loc, fst, temp); - cast = temp; + auto loadOp = mlir::cast(fst.getDefiningOp()); + mlir::Value originalStorage = loadOp.getMemref(); + cast = originalStorage; } else if (mlir::isa(snd) && mlir::isa(fst.getType())) { mlir::FunctionType funcTy = mlir::FunctionType::get(context, {}, {}); @@ -1654,10 +1653,9 @@ void prepareUserCallArguments( caller.placeInput(arg, value); else if (fir::isa_ref_type(argTy) && fir::dyn_cast_ptrEleTy(argTy) == value.getType()) { - // We're trying to convert value to reference - create temporary - mlir::Value temp = builder.createTemporary(loc, value.getType()); - builder.create(loc, value, temp); - caller.placeInput(arg, temp); + auto loadOp = mlir::cast(value.getDefiningOp()); + mlir::Value originalStorage = loadOp.getMemref(); + caller.placeInput(arg, originalStorage); } else caller.placeInput(arg, builder.createConvert(loc, argTy, value)); From b65fad21c576b138895d1fe77b493804a6a7094b Mon Sep 17 00:00:00 2001 From: Carlos Seo Date: Mon, 8 Sep 2025 15:18:37 +0000 Subject: [PATCH 3/4] Fix the testcase to reflect latest changes * Fix test to reflect the passing by loaded value change. * Check the actual call. * Use flang -fc1 -emit-hlfir instead of bbc. --- flang/test/Lower/percent-val-actual-argument.f90 | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/flang/test/Lower/percent-val-actual-argument.f90 b/flang/test/Lower/percent-val-actual-argument.f90 index 736baa0eca21d..890b1972e80bb 100644 --- a/flang/test/Lower/percent-val-actual-argument.f90 +++ b/flang/test/Lower/percent-val-actual-argument.f90 @@ -1,11 +1,13 @@ -! RUN: bbc %s -emit-fir -o - | FileCheck %s +! RUN: flang -fc1 -emit-hlfir %s -o - | FileCheck %s program main logical::a1 data a1/.true./ call sa(%val(a1)) -! CHECK: fir.load %3 : !fir.ref> -! CHECK: fir.convert %13 : (!fir.logical<4>) -> i1 +! CHECK: %[[A1_ADDR:.*]] = fir.address_of(@_QFEa1) : !fir.ref> +! CHECK: %[[A1_DECL:.*]]:2 = hlfir.declare %[[A1_ADDR]] {uniq_name = "_QFEa1"} : (!fir.ref>) -> (!fir.ref>, !fir.ref>) +! CHECK: fir.call @_QPsa(%[[A1_DECL]]#0) fastmath : (!fir.ref>) -> () +! CHECK: func.func @_QPsa(%[[SA_ARG:.*]]: !fir.ref> {fir.bindc_name = "x1"}) { write(6,*) "a1 = ", a1 end program main From cf32305fbcc48d08ce0ef980bffbb99c4cadee37 Mon Sep 17 00:00:00 2001 From: Carlos Seo Date: Mon, 8 Sep 2025 15:24:14 +0000 Subject: [PATCH 4/4] Add a new test to check passing %VAL to a VALUE argument --- flang/test/Lower/percent-val-value-argument.f90 | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 flang/test/Lower/percent-val-value-argument.f90 diff --git a/flang/test/Lower/percent-val-value-argument.f90 b/flang/test/Lower/percent-val-value-argument.f90 new file mode 100644 index 0000000000000..e7d5c5485c6c7 --- /dev/null +++ b/flang/test/Lower/percent-val-value-argument.f90 @@ -0,0 +1,17 @@ +! RUN: flang -fc1 -emit-hlfir %s -o - | FileCheck %s + +program main + logical::a1 + data a1/.true./ + call sb(%val(a1)) +! CHECK: %[[A1_ADDR:.*]] = fir.address_of(@_QFEa1) : !fir.ref> +! CHECK: %[[A1_DECL:.*]]:2 = hlfir.declare %[[A1_ADDR]] {uniq_name = "_QFEa1"} : (!fir.ref>) -> (!fir.ref>, !fir.ref>) +! CHECK: %[[A1_LOADED:.*]] = fir.load %[[A1_DECL]]#0 : !fir.ref> +! CHECK: fir.call @_QFPsb(%[[A1_LOADED]]) fastmath : (!fir.logical<4>) -> () +! CHECK: func.func private @_QFPsb(%[[SB_ARG:.*]]: !fir.logical<4> {fir.bindc_name = "x1"}) + write(6,*) "a1 = ", a1 +contains + subroutine sb(x1) + logical, value :: x1 + end subroutine sb +end program main