[flang-commits] [flang] [Flang] Handle %VAL arguments correctly (PR #157186)
Carlos Seo via flang-commits
flang-commits at lists.llvm.org
Mon Sep 8 08:25:51 PDT 2025
https://github.com/ceseo updated https://github.com/llvm/llvm-project/pull/157186
>From d7f5c644549df294e790a669154e00ccc1bdc2ef Mon Sep 17 00:00:00 2001
From: Carlos Seo <carlos.seo at linaro.org>
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<fir::BoxProcType>(snd) &&
- mlir::isa<mlir::FunctionType>(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<fir::StoreOp>(loc, fst, temp);
+ cast = temp;
+ } else if (mlir::isa<fir::BoxProcType>(snd) &&
+ mlir::isa<mlir::FunctionType>(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<mlir::Value>{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<fir::StoreOp>(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<!fir.logical<4>>
+! 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 <carlos.seo at linaro.org>
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<fir::StoreOp>(loc, fst, temp);
- cast = temp;
+ auto loadOp = mlir::cast<fir::LoadOp>(fst.getDefiningOp());
+ mlir::Value originalStorage = loadOp.getMemref();
+ cast = originalStorage;
} else if (mlir::isa<fir::BoxProcType>(snd) &&
mlir::isa<mlir::FunctionType>(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<fir::StoreOp>(loc, value, temp);
- caller.placeInput(arg, temp);
+ auto loadOp = mlir::cast<fir::LoadOp>(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 <carlos.seo at linaro.org>
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<!fir.logical<4>>
-! CHECK: fir.convert %13 : (!fir.logical<4>) -> i1
+! CHECK: %[[A1_ADDR:.*]] = fir.address_of(@_QFEa1) : !fir.ref<!fir.logical<4>>
+! CHECK: %[[A1_DECL:.*]]:2 = hlfir.declare %[[A1_ADDR]] {uniq_name = "_QFEa1"} : (!fir.ref<!fir.logical<4>>) -> (!fir.ref<!fir.logical<4>>, !fir.ref<!fir.logical<4>>)
+! CHECK: fir.call @_QPsa(%[[A1_DECL]]#0) fastmath<contract> : (!fir.ref<!fir.logical<4>>) -> ()
+! CHECK: func.func @_QPsa(%[[SA_ARG:.*]]: !fir.ref<!fir.logical<4>> {fir.bindc_name = "x1"}) {
write(6,*) "a1 = ", a1
end program main
>From cf32305fbcc48d08ce0ef980bffbb99c4cadee37 Mon Sep 17 00:00:00 2001
From: Carlos Seo <carlos.seo at linaro.org>
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<!fir.logical<4>>
+! CHECK: %[[A1_DECL:.*]]:2 = hlfir.declare %[[A1_ADDR]] {uniq_name = "_QFEa1"} : (!fir.ref<!fir.logical<4>>) -> (!fir.ref<!fir.logical<4>>, !fir.ref<!fir.logical<4>>)
+! CHECK: %[[A1_LOADED:.*]] = fir.load %[[A1_DECL]]#0 : !fir.ref<!fir.logical<4>>
+! CHECK: fir.call @_QFPsb(%[[A1_LOADED]]) fastmath<contract> : (!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
More information about the flang-commits
mailing list