From 632ed7be408faa9fba2e0db112fa1002c2f351b0 Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn Date: Mon, 29 Sep 2025 13:55:05 -0400 Subject: [PATCH 1/7] [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 Date: Tue, 30 Sep 2025 12:31:28 -0400 Subject: [PATCH 2/7] 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 Date: Tue, 30 Sep 2025 13:01:05 -0400 Subject: [PATCH 3/7] 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 Date: Tue, 30 Sep 2025 13:01:27 -0400 Subject: [PATCH 4/7] 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 Date: Tue, 30 Sep 2025 14:21:08 -0400 Subject: [PATCH 5/7] 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 Date: Tue, 30 Sep 2025 14:36:42 -0400 Subject: [PATCH 6/7] 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 Date: Tue, 30 Sep 2025 14:59:30 -0400 Subject: [PATCH 7/7] 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