[flang-commits] [flang] [flang] Fixed regression in copy-in/copy-out (PR #161259)
Eugene Epshteyn via flang-commits
flang-commits at lists.llvm.org
Mon Nov 10 10:03:29 PST 2025
https://github.com/eugeneepshteyn updated https://github.com/llvm/llvm-project/pull/161259
>From 632ed7be408faa9fba2e0db112fa1002c2f351b0 Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Mon, 29 Sep 2025 13:55:05 -0400
Subject: [PATCH 01/14] [flang] Fixed regression in copy-in/copy-out
Removed incorrect polymprphic check, added regression test.
Fixes #159149
---
flang/lib/Evaluate/check-expression.cpp | 33 -------------------------
flang/test/Lower/force-temp.f90 | 24 ++++++++++++++++++
2 files changed, 24 insertions(+), 33 deletions(-)
diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index 8931cbe485ac2..af3554c78b3ad 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -1493,36 +1493,6 @@ class CopyInOutExplicitInterface {
return !actualTreatAsContiguous && dummyNeedsContiguity;
}
- // Returns true, if actual and dummy have polymorphic differences
- bool HavePolymorphicDifferences() const {
- bool dummyIsAssumedRank{dummyObj_.type.attrs().test(
- characteristics::TypeAndShape::Attr::AssumedRank)};
- bool actualIsAssumedRank{semantics::IsAssumedRank(actual_)};
- bool dummyIsAssumedShape{dummyObj_.type.attrs().test(
- characteristics::TypeAndShape::Attr::AssumedShape)};
- bool actualIsAssumedShape{semantics::IsAssumedShape(actual_)};
- if ((actualIsAssumedRank && dummyIsAssumedRank) ||
- (actualIsAssumedShape && dummyIsAssumedShape)) {
- // Assumed-rank and assumed-shape arrays are represented by descriptors,
- // so don't need to do polymorphic check.
- } else if (!dummyObj_.ignoreTKR.test(common::IgnoreTKR::Type)) {
- // flang supports limited cases of passing polymorphic to non-polimorphic.
- // These cases require temporary of non-polymorphic type. (For example,
- // the actual argument could be polymorphic array of child type,
- // while the dummy argument could be non-polymorphic array of parent
- // type.)
- bool dummyIsPolymorphic{dummyObj_.type.type().IsPolymorphic()};
- auto actualType{
- characteristics::TypeAndShape::Characterize(actual_, fc_)};
- bool actualIsPolymorphic{
- actualType && actualType->type().IsPolymorphic()};
- if (actualIsPolymorphic && !dummyIsPolymorphic) {
- return true;
- }
- }
- return false;
- }
-
bool HaveArrayOrAssumedRankArgs() const {
bool dummyTreatAsArray{dummyObj_.ignoreTKR.test(common::IgnoreTKR::Rank)};
return IsArrayOrAssumedRank(actual_) &&
@@ -1611,9 +1581,6 @@ bool MayNeedCopy(const ActualArgument *actual,
if (check.HaveContiguityDifferences()) {
return true;
}
- if (check.HavePolymorphicDifferences()) {
- return true;
- }
} else { // Implicit interface
if (ExtractCoarrayRef(*actual)) {
// Coindexed actual args may need copy-in and copy-out with implicit
diff --git a/flang/test/Lower/force-temp.f90 b/flang/test/Lower/force-temp.f90
index d9ba543d46313..5a5e7d0abec5e 100644
--- a/flang/test/Lower/force-temp.f90
+++ b/flang/test/Lower/force-temp.f90
@@ -27,6 +27,14 @@ subroutine pass_intent_out(buf)
integer, intent(out) :: buf(5)
end subroutine
end interface
+
+ ! Used by s6() and call_s6()
+ type base
+ integer :: i = -1
+ end type
+ type, extends (base) :: child
+ real :: r = -2.0
+ end type
contains
subroutine s1(buf)
!CHECK-LABEL: func.func @_QMtestPs1
@@ -79,4 +87,20 @@ subroutine s5()
p => x(::2) ! pointer to non-contiguous array section
call pass_intent_out(p)
end subroutine
+ subroutine call_s6()
+!CHECK-LABEL: func.func @_QMtestPcall_s6
+!CHECK-NOT: hlfir.copy_in
+!CHECK: fir.call @_QMtestPs6
+!CHECK-NOT: hlfir.copy_out
+ class(base), pointer :: pb(:)
+ type(child), target :: c(2)
+
+ c = (/(child (i, real(i*2)), i=1,size(c))/)
+ pb => c
+ call s6(pb)
+ end subroutine call_s6
+ subroutine s6(b)
+ type(base), intent(inout) :: b(:)
+ b%i = 42
+ end subroutine s6
end module
>From 9189ede230e566442f5514a145e334b10d1255df Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Tue, 30 Sep 2025 12:31:28 -0400
Subject: [PATCH 02/14] Restore HavePolymorphicDifferences(), but with
different checks
---
flang/lib/Evaluate/check-expression.cpp | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index af3554c78b3ad..9337bb62387cc 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -1493,6 +1493,25 @@ class CopyInOutExplicitInterface {
return !actualTreatAsContiguous && dummyNeedsContiguity;
}
+ bool HavePolymorphicDifferences() const {
+ // These cases require temporary of non-polymorphic type. (For example,
+ // the actual argument could be polymorphic array of child type,
+ // while the dummy argument could be non-polymorphic array of parent
+ // type.)
+ if (dummyObj_.ignoreTKR.test(common::IgnoreTKR::Type)) {
+ return false;
+ }
+ auto actualType{characteristics::TypeAndShape::Characterize(actual_, fc_)};
+ bool actualIsPolymorphic{
+ actualType && actualType->type().IsPolymorphic()};
+ if (actualIsPolymorphic && !dummyObj_.IsPassedByDescriptor(/*isBindC*/false)) {
+ // Not passing a descriptor, so will need to make a copy of the data
+ // with a proper type.
+ return true;
+ }
+ return false;
+ }
+
bool HaveArrayOrAssumedRankArgs() const {
bool dummyTreatAsArray{dummyObj_.ignoreTKR.test(common::IgnoreTKR::Rank)};
return IsArrayOrAssumedRank(actual_) &&
@@ -1581,6 +1600,9 @@ bool MayNeedCopy(const ActualArgument *actual,
if (check.HaveContiguityDifferences()) {
return true;
}
+ if (check.HavePolymorphicDifferences()) {
+ return true;
+ }
} else { // Implicit interface
if (ExtractCoarrayRef(*actual)) {
// Coindexed actual args may need copy-in and copy-out with implicit
>From f572da2e5acee10a427306c14c998b7baee1de96 Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Tue, 30 Sep 2025 13:01:05 -0400
Subject: [PATCH 03/14] Tweaked the change to ignore assumed type in
polymorphic check
---
flang/lib/Evaluate/check-expression.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index 9337bb62387cc..60742f6aed7a7 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -1502,9 +1502,9 @@ class CopyInOutExplicitInterface {
return false;
}
auto actualType{characteristics::TypeAndShape::Characterize(actual_, fc_)};
- bool actualIsPolymorphic{
- actualType && actualType->type().IsPolymorphic()};
- if (actualIsPolymorphic && !dummyObj_.IsPassedByDescriptor(/*isBindC*/false)) {
+ if (actualType && actualType->type().IsPolymorphic() &&
+ !actualType->type().IsAssumedType() &&
+ !dummyObj_.IsPassedByDescriptor(/*isBindC*/false)) {
// Not passing a descriptor, so will need to make a copy of the data
// with a proper type.
return true;
>From 91f9e67bdef64f402cb2ca72a7d95d757ca59977 Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Tue, 30 Sep 2025 13:01:27 -0400
Subject: [PATCH 04/14] clang-format
---
flang/lib/Evaluate/check-expression.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index 60742f6aed7a7..b35fff70cabaf 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -1504,7 +1504,7 @@ class CopyInOutExplicitInterface {
auto actualType{characteristics::TypeAndShape::Characterize(actual_, fc_)};
if (actualType && actualType->type().IsPolymorphic() &&
!actualType->type().IsAssumedType() &&
- !dummyObj_.IsPassedByDescriptor(/*isBindC*/false)) {
+ !dummyObj_.IsPassedByDescriptor(/*isBindC*/ false)) {
// Not passing a descriptor, so will need to make a copy of the data
// with a proper type.
return true;
>From d60edf8b2251820f617a0e9bce942166ea651b88 Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Tue, 30 Sep 2025 14:21:08 -0400
Subject: [PATCH 05/14] Replaced s6() with an interface, since only care about
the call
---
flang/test/Lower/force-temp.f90 | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/flang/test/Lower/force-temp.f90 b/flang/test/Lower/force-temp.f90
index 5a5e7d0abec5e..cb06011f08ce2 100644
--- a/flang/test/Lower/force-temp.f90
+++ b/flang/test/Lower/force-temp.f90
@@ -88,9 +88,15 @@ subroutine s5()
call pass_intent_out(p)
end subroutine
subroutine call_s6()
+ interface
+ subroutine s6(b)
+ import :: base
+ type(base), intent(inout) :: b(:)
+ end subroutine s6
+ end interface
!CHECK-LABEL: func.func @_QMtestPcall_s6
!CHECK-NOT: hlfir.copy_in
-!CHECK: fir.call @_QMtestPs6
+!CHECK: fir.call @_QPs6
!CHECK-NOT: hlfir.copy_out
class(base), pointer :: pb(:)
type(child), target :: c(2)
@@ -99,8 +105,4 @@ subroutine call_s6()
pb => c
call s6(pb)
end subroutine call_s6
- subroutine s6(b)
- type(base), intent(inout) :: b(:)
- b%i = 42
- end subroutine s6
end module
>From 1e698038ba712c6d40ef32e384a69144d4304a4b Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Tue, 30 Sep 2025 14:36:42 -0400
Subject: [PATCH 06/14] New test to check for copy-in/out for potential slicing
test
---
flang/test/Lower/force-temp.f90 | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/flang/test/Lower/force-temp.f90 b/flang/test/Lower/force-temp.f90
index cb06011f08ce2..39d83c7c34a18 100644
--- a/flang/test/Lower/force-temp.f90
+++ b/flang/test/Lower/force-temp.f90
@@ -94,15 +94,32 @@ subroutine s6(b)
type(base), intent(inout) :: b(:)
end subroutine s6
end interface
+ class(base), pointer :: pb(:)
+ type(child), target :: c(2)
!CHECK-LABEL: func.func @_QMtestPcall_s6
!CHECK-NOT: hlfir.copy_in
!CHECK: fir.call @_QPs6
!CHECK-NOT: hlfir.copy_out
- class(base), pointer :: pb(:)
- type(child), target :: c(2)
-
- c = (/(child (i, real(i*2)), i=1,size(c))/)
pb => c
call s6(pb)
end subroutine call_s6
+ subroutine call_s7()
+ interface
+ subroutine s7(b1, b2, n)
+ import :: base
+ integer :: n
+ type(base), intent(inout) :: b1(n)
+ type(base), intent(inout) :: b2(*)
+ end subroutine
+ end interface
+ integer, parameter :: n = 7
+ class(base), allocatable :: c1(:), c2(:)
+!CHECK-LABEL: func.func @_QMtestPcall_s7
+!CHECK: hlfir.copy_in
+!CHECK: hlfir.copy_in
+!CHECK: fir.call @_QPs7
+!CHECK: hlfir.copy_out
+!CHECK: hlfir.copy_out
+ call s7(c1, c2, n)
+ end subroutine call_s7
end module
>From 94f69eda0ee72ab0dae48313f07d30bd5a3f74ee Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Tue, 30 Sep 2025 14:59:30 -0400
Subject: [PATCH 07/14] Tweaked test comment
---
flang/test/Lower/force-temp.f90 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/test/Lower/force-temp.f90 b/flang/test/Lower/force-temp.f90
index 39d83c7c34a18..e02463f700b45 100644
--- a/flang/test/Lower/force-temp.f90
+++ b/flang/test/Lower/force-temp.f90
@@ -28,7 +28,7 @@ subroutine pass_intent_out(buf)
end subroutine
end interface
- ! Used by s6() and call_s6()
+ ! Used by call_s6() and others below
type base
integer :: i = -1
end type
>From 5eff81517ae1921fc5f7f288f6f0cb491d72bfe4 Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Mon, 3 Nov 2025 11:03:43 -0500
Subject: [PATCH 08/14] Begin the switch to ternary logic using
std::optional<bool>. Renamed MayNeedCopy to ActualNeedsCopy
---
flang/include/flang/Evaluate/check-expression.h | 4 ++--
flang/lib/Evaluate/check-expression.cpp | 8 ++++----
flang/lib/Lower/ConvertCall.cpp | 10 ++++++----
flang/lib/Semantics/check-call.cpp | 7 ++++---
4 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/flang/include/flang/Evaluate/check-expression.h b/flang/include/flang/Evaluate/check-expression.h
index 2ff78d75325ef..34c7cabf7772c 100644
--- a/flang/include/flang/Evaluate/check-expression.h
+++ b/flang/include/flang/Evaluate/check-expression.h
@@ -163,8 +163,8 @@ extern template bool IsErrorExpr(const Expr<SomeType> &);
std::optional<parser::Message> CheckStatementFunction(
const Symbol &, const Expr<SomeType> &, FoldingContext &);
-bool MayNeedCopy(const ActualArgument *, const characteristics::DummyArgument *,
- FoldingContext &, bool forCopyOut);
+std::optional<bool> ActualNeedsCopy(const ActualArgument *,
+ const characteristics::DummyArgument *, FoldingContext &, bool forCopyOut);
} // namespace Fortran::evaluate
#endif
diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index fd7c047472e97..0db2a13ebbc2b 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -1565,14 +1565,14 @@ class CopyInOutExplicitInterface {
// perspective, meaning that for copy-in the caller need to do the copy
// before calling the callee. Similarly, for copy-out the caller is expected
// to do the copy after the callee returns.
-bool MayNeedCopy(const ActualArgument *actual,
+std::optional<bool> ActualNeedsCopy(const ActualArgument *actual,
const characteristics::DummyArgument *dummy, FoldingContext &fc,
bool forCopyOut) {
if (!actual) {
- return false;
+ return std::nullopt;
}
if (actual->isAlternateReturn()) {
- return false;
+ return std::nullopt;
}
const auto *dummyObj{dummy
? std::get_if<characteristics::DummyDataObject>(&dummy->u)
@@ -1603,7 +1603,7 @@ bool MayNeedCopy(const ActualArgument *actual,
// Note: contiguity and polymorphic checks deal with array or assumed rank
// arguments
if (!check.HaveArrayOrAssumedRankArgs()) {
- return false;
+ return std::nullopt;
}
if (check.HaveContiguityDifferences()) {
return true;
diff --git a/flang/lib/Lower/ConvertCall.cpp b/flang/lib/Lower/ConvertCall.cpp
index 9bf994e70cf5d..d6defff94d457 100644
--- a/flang/lib/Lower/ConvertCall.cpp
+++ b/flang/lib/Lower/ConvertCall.cpp
@@ -1296,10 +1296,12 @@ static PreparedDummyArgument preparePresentUserCallActualArgument(
Fortran::evaluate::FoldingContext &foldingContext{
callContext.converter.getFoldingContext()};
- bool suggestCopyIn = Fortran::evaluate::MayNeedCopy(
- arg.entity, arg.characteristics, foldingContext, /*forCopyOut=*/false);
- bool suggestCopyOut = Fortran::evaluate::MayNeedCopy(
- arg.entity, arg.characteristics, foldingContext, /*forCopyOut=*/true);
+ bool suggestCopyIn = Fortran::evaluate::ActualNeedsCopy(
+ arg.entity, arg.characteristics, foldingContext,
+ /*forCopyOut=*/false).value_or(false);
+ bool suggestCopyOut = Fortran::evaluate::ActualNeedsCopy(
+ arg.entity, arg.characteristics, foldingContext,
+ /*forCopyOut=*/true).value_or(false);
mustDoCopyIn = actual.isArray() && suggestCopyIn;
mustDoCopyOut = actual.isArray() && suggestCopyOut;
}
diff --git a/flang/lib/Semantics/check-call.cpp b/flang/lib/Semantics/check-call.cpp
index 995deaa12dd3b..a048f3a901643 100644
--- a/flang/lib/Semantics/check-call.cpp
+++ b/flang/lib/Semantics/check-call.cpp
@@ -795,7 +795,8 @@ static void CheckExplicitDataArg(const characteristics::DummyDataObject &dummy,
bool dummyIsAssumedShape{dummy.type.attrs().test(
characteristics::TypeAndShape::Attr::AssumedShape)};
bool copyOutNeeded{
- evaluate::MayNeedCopy(&arg, &dummyArg, foldingContext, true)};
+ evaluate::ActualNeedsCopy(&arg, &dummyArg, foldingContext,
+ /*forCopyOut=*/true).value_or(false)};
if (copyOutNeeded && !dummyIsValue &&
(dummyIsAsynchronous || dummyIsVolatile)) {
if (actualIsAsynchronous || actualIsVolatile) {
@@ -832,8 +833,8 @@ static void CheckExplicitDataArg(const characteristics::DummyDataObject &dummy,
// a unread value in the actual argument.
// Occurences of `volatileOrAsyncNeedsTempDiagnosticIssued = true` indicate a
// more specific error message has already been issued. We might be able to
- // clean this up by switching the coding style of MayNeedCopy to be more like
- // WhyNotDefinable.
+ // clean this up by switching the coding style of ActualNeedsCopy to be more
+ // like WhyNotDefinable.
if (copyOutNeeded && !volatileOrAsyncNeedsTempDiagnosticIssued) {
if ((actualIsVolatile || actualIsAsynchronous) &&
(dummyIsVolatile || dummyIsAsynchronous)) {
>From 68bab542f84cb0e6d1ae16ce65e73a689cd89486 Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Mon, 3 Nov 2025 11:53:49 -0500
Subject: [PATCH 09/14] New name: ActualArgNeedsCopy(). Reworked the implicit
interface case
---
.../include/flang/Evaluate/check-expression.h | 2 +-
flang/lib/Evaluate/check-expression.cpp | 32 ++++++++++---------
flang/lib/Lower/ConvertCall.cpp | 4 +--
flang/lib/Semantics/check-call.cpp | 6 ++--
4 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/flang/include/flang/Evaluate/check-expression.h b/flang/include/flang/Evaluate/check-expression.h
index 34c7cabf7772c..d11fe22c0be7b 100644
--- a/flang/include/flang/Evaluate/check-expression.h
+++ b/flang/include/flang/Evaluate/check-expression.h
@@ -163,7 +163,7 @@ extern template bool IsErrorExpr(const Expr<SomeType> &);
std::optional<parser::Message> CheckStatementFunction(
const Symbol &, const Expr<SomeType> &, FoldingContext &);
-std::optional<bool> ActualNeedsCopy(const ActualArgument *,
+std::optional<bool> ActualArgNeedsCopy(const ActualArgument *,
const characteristics::DummyArgument *, FoldingContext &, bool forCopyOut);
} // namespace Fortran::evaluate
diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index 0db2a13ebbc2b..a3d3171c451e7 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -1565,14 +1565,15 @@ class CopyInOutExplicitInterface {
// perspective, meaning that for copy-in the caller need to do the copy
// before calling the callee. Similarly, for copy-out the caller is expected
// to do the copy after the callee returns.
-std::optional<bool> ActualNeedsCopy(const ActualArgument *actual,
+std::optional<bool> ActualArgNeedsCopy(const ActualArgument *actual,
const characteristics::DummyArgument *dummy, FoldingContext &fc,
bool forCopyOut) {
+ constexpr auto unknown = std::nullopt;
if (!actual) {
- return std::nullopt;
+ return unknown;
}
if (actual->isAlternateReturn()) {
- return std::nullopt;
+ return unknown;
}
const auto *dummyObj{dummy
? std::get_if<characteristics::DummyDataObject>(&dummy->u)
@@ -1603,7 +1604,7 @@ std::optional<bool> ActualNeedsCopy(const ActualArgument *actual,
// Note: contiguity and polymorphic checks deal with array or assumed rank
// arguments
if (!check.HaveArrayOrAssumedRankArgs()) {
- return std::nullopt;
+ return unknown;
}
if (check.HaveContiguityDifferences()) {
return true;
@@ -1612,21 +1613,22 @@ std::optional<bool> ActualNeedsCopy(const ActualArgument *actual,
return true;
}
} else { // Implicit interface
- if (ExtractCoarrayRef(*actual)) {
- // Coindexed actual args may need copy-in and copy-out with implicit
- // interface
+ bool hasVectorSubscript{HasVectorSubscript(*actual)};
+ if (forCopyOut && hasVectorSubscript) {
+ // Vector subscripts could refer to duplicate elments, can't copy out
+ return false;
+ }
+ if (forCopyIn && hasVectorSubscript) {
return true;
}
- if (!IsSimplyContiguous(*actual, fc)) {
- // Copy-in: actual arguments that are variables are copy-in when
- // non-contiguous.
- // Copy-out: vector subscripts could refer to duplicate elements, can't
- // copy out.
- return !(forCopyOut && HasVectorSubscript(*actual));
+ if (auto isContig{IsContiguous(*actual, fc)}) {
+ // If we are pretty sure the actual argument is contiguous, then we
+ // don't need to copy it. On the other hand, if we are pretty sure the
+ // actual argument is not contiguous, then we need to copy it.
+ return !isContig.value();
}
}
- // For everything else, no copy-in or copy-out
- return false;
+ return unknown;
}
} // namespace Fortran::evaluate
diff --git a/flang/lib/Lower/ConvertCall.cpp b/flang/lib/Lower/ConvertCall.cpp
index d6defff94d457..447d3c6864506 100644
--- a/flang/lib/Lower/ConvertCall.cpp
+++ b/flang/lib/Lower/ConvertCall.cpp
@@ -1296,10 +1296,10 @@ static PreparedDummyArgument preparePresentUserCallActualArgument(
Fortran::evaluate::FoldingContext &foldingContext{
callContext.converter.getFoldingContext()};
- bool suggestCopyIn = Fortran::evaluate::ActualNeedsCopy(
+ bool suggestCopyIn = Fortran::evaluate::ActualArgNeedsCopy(
arg.entity, arg.characteristics, foldingContext,
/*forCopyOut=*/false).value_or(false);
- bool suggestCopyOut = Fortran::evaluate::ActualNeedsCopy(
+ bool suggestCopyOut = Fortran::evaluate::ActualArgNeedsCopy(
arg.entity, arg.characteristics, foldingContext,
/*forCopyOut=*/true).value_or(false);
mustDoCopyIn = actual.isArray() && suggestCopyIn;
diff --git a/flang/lib/Semantics/check-call.cpp b/flang/lib/Semantics/check-call.cpp
index a048f3a901643..327dd306ff583 100644
--- a/flang/lib/Semantics/check-call.cpp
+++ b/flang/lib/Semantics/check-call.cpp
@@ -795,7 +795,7 @@ static void CheckExplicitDataArg(const characteristics::DummyDataObject &dummy,
bool dummyIsAssumedShape{dummy.type.attrs().test(
characteristics::TypeAndShape::Attr::AssumedShape)};
bool copyOutNeeded{
- evaluate::ActualNeedsCopy(&arg, &dummyArg, foldingContext,
+ evaluate::ActualArgNeedsCopy(&arg, &dummyArg, foldingContext,
/*forCopyOut=*/true).value_or(false)};
if (copyOutNeeded && !dummyIsValue &&
(dummyIsAsynchronous || dummyIsVolatile)) {
@@ -833,8 +833,8 @@ static void CheckExplicitDataArg(const characteristics::DummyDataObject &dummy,
// a unread value in the actual argument.
// Occurences of `volatileOrAsyncNeedsTempDiagnosticIssued = true` indicate a
// more specific error message has already been issued. We might be able to
- // clean this up by switching the coding style of ActualNeedsCopy to be more
- // like WhyNotDefinable.
+ // clean this up by switching the coding style of ActualArgNeedsCopy to be
+ // more like WhyNotDefinable.
if (copyOutNeeded && !volatileOrAsyncNeedsTempDiagnosticIssued) {
if ((actualIsVolatile || actualIsAsynchronous) &&
(dummyIsVolatile || dummyIsAsynchronous)) {
>From 413718c278a626a20593c929a8c52beb683869f7 Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Mon, 3 Nov 2025 21:02:26 -0500
Subject: [PATCH 10/14] Continue refactoring. Extracted DummyNeedsContiguity()
functionality into separate function
---
flang/lib/Evaluate/check-expression.cpp | 38 ++++++++++---------------
1 file changed, 15 insertions(+), 23 deletions(-)
diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index a3d3171c451e7..12050eed92445 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -1475,13 +1475,9 @@ class CopyInOutExplicitInterface {
const characteristics::DummyDataObject &dummyObj)
: fc_{fc}, actual_{actual}, dummyObj_{dummyObj} {}
- // Returns true, if actual and dummy have different contiguity requirements
- bool HaveContiguityDifferences() const {
- // Check actual contiguity, unless dummy doesn't care
+ // Returns true if dummy arg needs to be contiguous
+ bool DummyNeedsContiguity() const {
bool dummyTreatAsArray{dummyObj_.ignoreTKR.test(common::IgnoreTKR::Rank)};
- bool actualTreatAsContiguous{
- dummyObj_.ignoreTKR.test(common::IgnoreTKR::Contiguous) ||
- IsSimplyContiguous(actual_, fc_)};
bool dummyIsExplicitShape{dummyObj_.type.IsExplicitShape()};
bool dummyIsAssumedSize{dummyObj_.type.attrs().test(
characteristics::TypeAndShape::Attr::AssumedSize)};
@@ -1498,7 +1494,7 @@ class CopyInOutExplicitInterface {
(dummyTreatAsArray && !dummyIsPolymorphic) || dummyIsVoidStar ||
dummyObj_.attrs.test(
characteristics::DummyDataObject::Attr::Contiguous)};
- return !actualTreatAsContiguous && dummyNeedsContiguity;
+ return dummyNeedsContiguity;
}
bool HavePolymorphicDifferences() const {
@@ -1561,6 +1557,10 @@ class CopyInOutExplicitInterface {
// procedures with explicit interface, it's expected that "dummy" is not null.
// For procedures with implicit interface dummy may be null.
//
+// Returns std::optional<bool> indicating whether the copy is known to be
+// needed (true) or not needed (false); returns std::nullopt if the necessity
+// of the copy is undetermined.
+//
// Note that these copy-in and copy-out checks are done from the caller's
// perspective, meaning that for copy-in the caller need to do the copy
// before calling the callee. Similarly, for copy-out the caller is expected
@@ -1580,8 +1580,7 @@ std::optional<bool> ActualArgNeedsCopy(const ActualArgument *actual,
: nullptr};
const bool forCopyIn = !forCopyOut;
if (!evaluate::IsVariable(*actual)) {
- // Actual argument expressions that aren’t variables are copy-in, but
- // not copy-out.
+ // Expressions are copy-in, but not copy-out.
return forCopyIn;
}
if (dummyObj) { // Explict interface
@@ -1604,29 +1603,22 @@ std::optional<bool> ActualArgNeedsCopy(const ActualArgument *actual,
// Note: contiguity and polymorphic checks deal with array or assumed rank
// arguments
if (!check.HaveArrayOrAssumedRankArgs()) {
- return unknown;
+ return false;
}
- if (check.HaveContiguityDifferences()) {
+ bool actualTreatAsContiguous{
+ dummyObj->ignoreTKR.test(common::IgnoreTKR::Contiguous) ||
+ IsSimplyContiguous(*actual, fc)};
+ if (!actualTreatAsContiguous && check.DummyNeedsContiguity()) {
return true;
}
if (check.HavePolymorphicDifferences()) {
return true;
}
} else { // Implicit interface
- bool hasVectorSubscript{HasVectorSubscript(*actual)};
- if (forCopyOut && hasVectorSubscript) {
- // Vector subscripts could refer to duplicate elments, can't copy out
+ if (auto maybeContig{IsContiguous(*actual, fc)}; *maybeContig) {
+ // Known contiguous, don't copy in/out
return false;
}
- if (forCopyIn && hasVectorSubscript) {
- return true;
- }
- if (auto isContig{IsContiguous(*actual, fc)}) {
- // If we are pretty sure the actual argument is contiguous, then we
- // don't need to copy it. On the other hand, if we are pretty sure the
- // actual argument is not contiguous, then we need to copy it.
- return !isContig.value();
- }
}
return unknown;
}
>From 329445cfc16f9142b8345026a81c4ac4e9574c9f Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Mon, 3 Nov 2025 21:20:25 -0500
Subject: [PATCH 11/14] Continue refactoring
---
flang/lib/Evaluate/check-expression.cpp | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index 12050eed92445..5ffe299a51f97 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -1583,6 +1583,8 @@ std::optional<bool> ActualArgNeedsCopy(const ActualArgument *actual,
// Expressions are copy-in, but not copy-out.
return forCopyIn;
}
+ auto maybeContigActual{IsContiguous(*actual, fc)};
+ bool isContiguousActual{maybeContigActual.has_value() && maybeContigActual.value()};
if (dummyObj) { // Explict interface
CopyInOutExplicitInterface check{fc, *actual, *dummyObj};
if (forCopyOut && check.HasIntentIn()) {
@@ -1605,9 +1607,8 @@ std::optional<bool> ActualArgNeedsCopy(const ActualArgument *actual,
if (!check.HaveArrayOrAssumedRankArgs()) {
return false;
}
- bool actualTreatAsContiguous{
- dummyObj->ignoreTKR.test(common::IgnoreTKR::Contiguous) ||
- IsSimplyContiguous(*actual, fc)};
+ bool actualTreatAsContiguous{isContiguousActual ||
+ dummyObj->ignoreTKR.test(common::IgnoreTKR::Contiguous)};
if (!actualTreatAsContiguous && check.DummyNeedsContiguity()) {
return true;
}
@@ -1615,7 +1616,7 @@ std::optional<bool> ActualArgNeedsCopy(const ActualArgument *actual,
return true;
}
} else { // Implicit interface
- if (auto maybeContig{IsContiguous(*actual, fc)}; *maybeContig) {
+ if (isContiguousActual) {
// Known contiguous, don't copy in/out
return false;
}
>From ad080f186a4182c1d33e4b699f2d08320d20b3ff Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Mon, 3 Nov 2025 21:25:14 -0500
Subject: [PATCH 12/14] clang-format
---
flang/lib/Evaluate/check-expression.cpp | 3 ++-
flang/lib/Lower/ConvertCall.cpp | 10 ++++++----
flang/lib/Semantics/check-call.cpp | 3 ++-
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index 5ffe299a51f97..0899cd3011c01 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -1584,7 +1584,8 @@ std::optional<bool> ActualArgNeedsCopy(const ActualArgument *actual,
return forCopyIn;
}
auto maybeContigActual{IsContiguous(*actual, fc)};
- bool isContiguousActual{maybeContigActual.has_value() && maybeContigActual.value()};
+ bool isContiguousActual{
+ maybeContigActual.has_value() && maybeContigActual.value()};
if (dummyObj) { // Explict interface
CopyInOutExplicitInterface check{fc, *actual, *dummyObj};
if (forCopyOut && check.HasIntentIn()) {
diff --git a/flang/lib/Lower/ConvertCall.cpp b/flang/lib/Lower/ConvertCall.cpp
index 447d3c6864506..bb4d1d707c60a 100644
--- a/flang/lib/Lower/ConvertCall.cpp
+++ b/flang/lib/Lower/ConvertCall.cpp
@@ -1297,11 +1297,13 @@ static PreparedDummyArgument preparePresentUserCallActualArgument(
callContext.converter.getFoldingContext()};
bool suggestCopyIn = Fortran::evaluate::ActualArgNeedsCopy(
- arg.entity, arg.characteristics, foldingContext,
- /*forCopyOut=*/false).value_or(false);
+ arg.entity, arg.characteristics, foldingContext,
+ /*forCopyOut=*/false)
+ .value_or(false);
bool suggestCopyOut = Fortran::evaluate::ActualArgNeedsCopy(
- arg.entity, arg.characteristics, foldingContext,
- /*forCopyOut=*/true).value_or(false);
+ arg.entity, arg.characteristics, foldingContext,
+ /*forCopyOut=*/true)
+ .value_or(false);
mustDoCopyIn = actual.isArray() && suggestCopyIn;
mustDoCopyOut = actual.isArray() && suggestCopyOut;
}
diff --git a/flang/lib/Semantics/check-call.cpp b/flang/lib/Semantics/check-call.cpp
index 327dd306ff583..4b0f62c3881a1 100644
--- a/flang/lib/Semantics/check-call.cpp
+++ b/flang/lib/Semantics/check-call.cpp
@@ -796,7 +796,8 @@ static void CheckExplicitDataArg(const characteristics::DummyDataObject &dummy,
characteristics::TypeAndShape::Attr::AssumedShape)};
bool copyOutNeeded{
evaluate::ActualArgNeedsCopy(&arg, &dummyArg, foldingContext,
- /*forCopyOut=*/true).value_or(false)};
+ /*forCopyOut=*/true)
+ .value_or(false)};
if (copyOutNeeded && !dummyIsValue &&
(dummyIsAsynchronous || dummyIsVolatile)) {
if (actualIsAsynchronous || actualIsVolatile) {
>From 8792ba42cd59c43f6d554746cf721d9b4fa0940a Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Tue, 4 Nov 2025 18:04:31 -0500
Subject: [PATCH 13/14] Simplified check.HavePolymorphicDifferences(). Now
seems to work
---
flang/lib/Evaluate/check-expression.cpp | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index 0899cd3011c01..d9c91f23456bc 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -1498,20 +1498,15 @@ class CopyInOutExplicitInterface {
}
bool HavePolymorphicDifferences() const {
- // These cases require temporary of non-polymorphic type. (For example,
- // the actual argument could be polymorphic array of child type,
- // while the dummy argument could be non-polymorphic array of parent
- // type.)
if (dummyObj_.ignoreTKR.test(common::IgnoreTKR::Type)) {
return false;
}
- auto actualType{characteristics::TypeAndShape::Characterize(actual_, fc_)};
- if (actualType && actualType->type().IsPolymorphic() &&
- !actualType->type().IsAssumedType() &&
- !dummyObj_.IsPassedByDescriptor(/*isBindC*/ false)) {
- // Not passing a descriptor, so will need to make a copy of the data
- // with a proper type.
- return true;
+ if (auto actualType{characteristics::TypeAndShape::Characterize(actual_, fc_)}) {
+ bool actualIsPolymorphic{actualType->type().IsPolymorphic()};
+ bool dummyIsPolymorphic{dummyObj_.type.type().IsPolymorphic()};
+ if (actualIsPolymorphic && !dummyIsPolymorphic) {
+ return true;
+ }
}
return false;
}
@@ -1610,10 +1605,8 @@ std::optional<bool> ActualArgNeedsCopy(const ActualArgument *actual,
}
bool actualTreatAsContiguous{isContiguousActual ||
dummyObj->ignoreTKR.test(common::IgnoreTKR::Contiguous)};
- if (!actualTreatAsContiguous && check.DummyNeedsContiguity()) {
- return true;
- }
- if (check.HavePolymorphicDifferences()) {
+ if ((!actualTreatAsContiguous || check.HavePolymorphicDifferences()) &&
+ check.DummyNeedsContiguity()) {
return true;
}
} else { // Implicit interface
>From 8fd94273b12541714d3bbda5cb5fceecc3168de8 Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Tue, 4 Nov 2025 18:04:57 -0500
Subject: [PATCH 14/14] clang-format
---
flang/lib/Evaluate/check-expression.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index d9c91f23456bc..30db398cbeb46 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -1501,7 +1501,8 @@ class CopyInOutExplicitInterface {
if (dummyObj_.ignoreTKR.test(common::IgnoreTKR::Type)) {
return false;
}
- if (auto actualType{characteristics::TypeAndShape::Characterize(actual_, fc_)}) {
+ if (auto actualType{
+ characteristics::TypeAndShape::Characterize(actual_, fc_)}) {
bool actualIsPolymorphic{actualType->type().IsPolymorphic()};
bool dummyIsPolymorphic{dummyObj_.type.type().IsPolymorphic()};
if (actualIsPolymorphic && !dummyIsPolymorphic) {
More information about the flang-commits
mailing list