Skip to content

[flang] Accept CLASS(*) array in EOSHIFT #116114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 14, 2024
Merged

Conversation

klausler
Copy link
Contributor

The intrinsic processing code wasn't allowing the ARRAY= argument to the EOSHIFT intrinsic function to be CLASS(*). That case seems to conform to the standard, although only one compiler could actually handle it, so allow for it.

Fixes #115923.

@klausler klausler requested a review from DanielCChen November 13, 2024 22:35
@llvmbot llvmbot added flang:runtime flang Flang issues not falling into any other category flang:semantics labels Nov 13, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 13, 2024

@llvm/pr-subscribers-flang-semantics

Author: Peter Klausler (klausler)

Changes

The intrinsic processing code wasn't allowing the ARRAY= argument to the EOSHIFT intrinsic function to be CLASS(*). That case seems to conform to the standard, although only one compiler could actually handle it, so allow for it.

Fixes #115923.


Full diff: https://github.com/llvm/llvm-project/pull/116114.diff

3 Files Affected:

  • (modified) flang/lib/Evaluate/intrinsics.cpp (+7-9)
  • (modified) flang/runtime/transformational.cpp (+7-7)
  • (added) flang/test/Evaluate/bug115923.f90 (+22)
diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp
index aa44967817722e..5290fd2e7210cc 100644
--- a/flang/lib/Evaluate/intrinsics.cpp
+++ b/flang/lib/Evaluate/intrinsics.cpp
@@ -438,6 +438,12 @@ static const IntrinsicInterface genericIntrinsicFunction[]{
             {"shift", AnyInt}},
         SameInt},
     {"dshiftr", {{"i", BOZ}, {"j", SameInt}, {"shift", AnyInt}}, SameInt},
+    {"eoshift",
+        {{"array", SameType, Rank::array},
+            {"shift", AnyInt, Rank::dimRemovedOrScalar},
+            // BOUNDARY= is not optional for non-intrinsic types
+            {"boundary", SameType, Rank::dimRemovedOrScalar}, OptionalDIM},
+        SameType, Rank::conformable, IntrinsicClass::transformationalFunction},
     {"eoshift",
         {{"array", SameIntrinsic, Rank::array},
             {"shift", AnyInt, Rank::dimRemovedOrScalar},
@@ -446,14 +452,6 @@ static const IntrinsicInterface genericIntrinsicFunction[]{
             OptionalDIM},
         SameIntrinsic, Rank::conformable,
         IntrinsicClass::transformationalFunction},
-    {"eoshift",
-        {{"array", SameDerivedType, Rank::array},
-            {"shift", AnyInt, Rank::dimRemovedOrScalar},
-            // BOUNDARY= is not optional for derived types
-            {"boundary", SameDerivedType, Rank::dimRemovedOrScalar},
-            OptionalDIM},
-        SameDerivedType, Rank::conformable,
-        IntrinsicClass::transformationalFunction},
     {"epsilon",
         {{"x", SameReal, Rank::anyOrAssumedRank, Optionality::required,
             common::Intent::In, {ArgFlag::canBeMoldNull}}},
@@ -1943,7 +1941,7 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
       if (!sameArg) {
         sameArg = arg;
       }
-      argOk = type->IsTkLenCompatibleWith(sameArg->GetType().value());
+      argOk = sameArg->GetType().value().IsTkLenCompatibleWith(*type);
       break;
     case KindCode::sameKind:
       if (!sameArg) {
diff --git a/flang/runtime/transformational.cpp b/flang/runtime/transformational.cpp
index b65502933b862f..ab303bdef9b1d1 100644
--- a/flang/runtime/transformational.cpp
+++ b/flang/runtime/transformational.cpp
@@ -46,7 +46,7 @@ class ShiftControl {
           lb_[k++] = shiftDim.LowerBound();
           if (shiftDim.Extent() != source.GetDimension(j).Extent()) {
             terminator_.Crash("%s: on dimension %d, SHIFT= has extent %jd but "
-                              "SOURCE= has extent %jd",
+                              "ARRAY= has extent %jd",
                 which, k, static_cast<std::intmax_t>(shiftDim.Extent()),
                 static_cast<std::intmax_t>(source.GetDimension(j).Extent()));
           }
@@ -460,7 +460,7 @@ void RTDEF(Cshift)(Descriptor &result, const Descriptor &source,
   RUNTIME_CHECK(terminator, rank > 1);
   if (dim < 1 || dim > rank) {
     terminator.Crash(
-        "CSHIFT: DIM=%d must be >= 1 and <= SOURCE= rank %d", dim, rank);
+        "CSHIFT: DIM=%d must be >= 1 and <= ARRAY= rank %d", dim, rank);
   }
   ShiftControl shiftControl{shift, terminator, dim};
   shiftControl.Init(source, "CSHIFT");
@@ -527,7 +527,7 @@ void RTDEF(Eoshift)(Descriptor &result, const Descriptor &source,
   RUNTIME_CHECK(terminator, rank > 1);
   if (dim < 1 || dim > rank) {
     terminator.Crash(
-        "EOSHIFT: DIM=%d must be >= 1 and <= SOURCE= rank %d", dim, rank);
+        "EOSHIFT: DIM=%d must be >= 1 and <= ARRAY= rank %d", dim, rank);
   }
   std::size_t elementLen{
       AllocateResult(result, source, rank, extent, terminator, "EOSHIFT")};
@@ -538,7 +538,7 @@ void RTDEF(Eoshift)(Descriptor &result, const Descriptor &source,
     RUNTIME_CHECK(terminator, boundary->type() == source.type());
     if (boundary->ElementBytes() != elementLen) {
       terminator.Crash("EOSHIFT: BOUNDARY= has element byte length %zd, but "
-                       "SOURCE= has length %zd",
+                       "ARRAY= has length %zd",
           boundary->ElementBytes(), elementLen);
     }
     if (boundaryRank > 0) {
@@ -547,7 +547,7 @@ void RTDEF(Eoshift)(Descriptor &result, const Descriptor &source,
         if (j != dim - 1) {
           if (boundary->GetDimension(k).Extent() != extent[j]) {
             terminator.Crash("EOSHIFT: BOUNDARY= has extent %jd on dimension "
-                             "%d but must conform with extent %jd of SOURCE=",
+                             "%d but must conform with extent %jd of ARRAY=",
                 static_cast<std::intmax_t>(boundary->GetDimension(k).Extent()),
                 k + 1, static_cast<std::intmax_t>(extent[j]));
           }
@@ -611,7 +611,7 @@ void RTDEF(EoshiftVector)(Descriptor &result, const Descriptor &source,
     RUNTIME_CHECK(terminator, boundary->type() == source.type());
     if (boundary->ElementBytes() != elementLen) {
       terminator.Crash("EOSHIFT: BOUNDARY= has element byte length %zd but "
-                       "SOURCE= has length %zd",
+                       "ARRAY= has length %zd",
           boundary->ElementBytes(), elementLen);
     }
   }
@@ -658,7 +658,7 @@ void RTDEF(Pack)(Descriptor &result, const Descriptor &source,
     RUNTIME_CHECK(terminator, vector->rank() == 1);
     RUNTIME_CHECK(terminator, source.type() == vector->type());
     if (source.ElementBytes() != vector->ElementBytes()) {
-      terminator.Crash("PACK: SOURCE= has element byte length %zd, but VECTOR= "
+      terminator.Crash("PACK: ARRAY= has element byte length %zd, but VECTOR= "
                        "has length %zd",
           source.ElementBytes(), vector->ElementBytes());
     }
diff --git a/flang/test/Evaluate/bug115923.f90 b/flang/test/Evaluate/bug115923.f90
new file mode 100644
index 00000000000000..c8cbaed1d254e9
--- /dev/null
+++ b/flang/test/Evaluate/bug115923.f90
@@ -0,0 +1,22 @@
+! RUN: %flang_fc1 -fsyntax-only -pedantic -Werror 2>&1 | FileCheck --allow-empty %s
+! Ensure that EOSHIFT's ARRAY= argument and result can be CLASS(*).
+! CHECK-NOT: error:
+! CHECK: warning: Source of TRANSFER is polymorphic
+! CHECK: warning: Mold of TRANSFER is polymorphic
+program p
+  type base
+    integer j
+  end type
+  type, extends(base) :: extended
+    integer k
+  end type
+  class(base), allocatable :: polyArray(:,:,:)
+  class(*), allocatable :: unlimited(:)
+  allocate(polyArray, source=reshape([(extended(n,n-1),n=1,8)],[2,2,2]))
+  allocate(unlimited, source=[(base(9),n=1,16)])
+  select type (x => eoshift(transfer(polyArray, unlimited), -4, base(-1)))
+    type is (base); print *, 'base', x
+    type is (extended); print *, 'extended?', x
+    class default; print *, 'class default??'
+  end select
+end

@llvmbot
Copy link
Member

llvmbot commented Nov 13, 2024

@llvm/pr-subscribers-flang-runtime

Author: Peter Klausler (klausler)

Changes

The intrinsic processing code wasn't allowing the ARRAY= argument to the EOSHIFT intrinsic function to be CLASS(*). That case seems to conform to the standard, although only one compiler could actually handle it, so allow for it.

Fixes #115923.


Full diff: https://github.com/llvm/llvm-project/pull/116114.diff

3 Files Affected:

  • (modified) flang/lib/Evaluate/intrinsics.cpp (+7-9)
  • (modified) flang/runtime/transformational.cpp (+7-7)
  • (added) flang/test/Evaluate/bug115923.f90 (+22)
diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp
index aa44967817722e..5290fd2e7210cc 100644
--- a/flang/lib/Evaluate/intrinsics.cpp
+++ b/flang/lib/Evaluate/intrinsics.cpp
@@ -438,6 +438,12 @@ static const IntrinsicInterface genericIntrinsicFunction[]{
             {"shift", AnyInt}},
         SameInt},
     {"dshiftr", {{"i", BOZ}, {"j", SameInt}, {"shift", AnyInt}}, SameInt},
+    {"eoshift",
+        {{"array", SameType, Rank::array},
+            {"shift", AnyInt, Rank::dimRemovedOrScalar},
+            // BOUNDARY= is not optional for non-intrinsic types
+            {"boundary", SameType, Rank::dimRemovedOrScalar}, OptionalDIM},
+        SameType, Rank::conformable, IntrinsicClass::transformationalFunction},
     {"eoshift",
         {{"array", SameIntrinsic, Rank::array},
             {"shift", AnyInt, Rank::dimRemovedOrScalar},
@@ -446,14 +452,6 @@ static const IntrinsicInterface genericIntrinsicFunction[]{
             OptionalDIM},
         SameIntrinsic, Rank::conformable,
         IntrinsicClass::transformationalFunction},
-    {"eoshift",
-        {{"array", SameDerivedType, Rank::array},
-            {"shift", AnyInt, Rank::dimRemovedOrScalar},
-            // BOUNDARY= is not optional for derived types
-            {"boundary", SameDerivedType, Rank::dimRemovedOrScalar},
-            OptionalDIM},
-        SameDerivedType, Rank::conformable,
-        IntrinsicClass::transformationalFunction},
     {"epsilon",
         {{"x", SameReal, Rank::anyOrAssumedRank, Optionality::required,
             common::Intent::In, {ArgFlag::canBeMoldNull}}},
@@ -1943,7 +1941,7 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
       if (!sameArg) {
         sameArg = arg;
       }
-      argOk = type->IsTkLenCompatibleWith(sameArg->GetType().value());
+      argOk = sameArg->GetType().value().IsTkLenCompatibleWith(*type);
       break;
     case KindCode::sameKind:
       if (!sameArg) {
diff --git a/flang/runtime/transformational.cpp b/flang/runtime/transformational.cpp
index b65502933b862f..ab303bdef9b1d1 100644
--- a/flang/runtime/transformational.cpp
+++ b/flang/runtime/transformational.cpp
@@ -46,7 +46,7 @@ class ShiftControl {
           lb_[k++] = shiftDim.LowerBound();
           if (shiftDim.Extent() != source.GetDimension(j).Extent()) {
             terminator_.Crash("%s: on dimension %d, SHIFT= has extent %jd but "
-                              "SOURCE= has extent %jd",
+                              "ARRAY= has extent %jd",
                 which, k, static_cast<std::intmax_t>(shiftDim.Extent()),
                 static_cast<std::intmax_t>(source.GetDimension(j).Extent()));
           }
@@ -460,7 +460,7 @@ void RTDEF(Cshift)(Descriptor &result, const Descriptor &source,
   RUNTIME_CHECK(terminator, rank > 1);
   if (dim < 1 || dim > rank) {
     terminator.Crash(
-        "CSHIFT: DIM=%d must be >= 1 and <= SOURCE= rank %d", dim, rank);
+        "CSHIFT: DIM=%d must be >= 1 and <= ARRAY= rank %d", dim, rank);
   }
   ShiftControl shiftControl{shift, terminator, dim};
   shiftControl.Init(source, "CSHIFT");
@@ -527,7 +527,7 @@ void RTDEF(Eoshift)(Descriptor &result, const Descriptor &source,
   RUNTIME_CHECK(terminator, rank > 1);
   if (dim < 1 || dim > rank) {
     terminator.Crash(
-        "EOSHIFT: DIM=%d must be >= 1 and <= SOURCE= rank %d", dim, rank);
+        "EOSHIFT: DIM=%d must be >= 1 and <= ARRAY= rank %d", dim, rank);
   }
   std::size_t elementLen{
       AllocateResult(result, source, rank, extent, terminator, "EOSHIFT")};
@@ -538,7 +538,7 @@ void RTDEF(Eoshift)(Descriptor &result, const Descriptor &source,
     RUNTIME_CHECK(terminator, boundary->type() == source.type());
     if (boundary->ElementBytes() != elementLen) {
       terminator.Crash("EOSHIFT: BOUNDARY= has element byte length %zd, but "
-                       "SOURCE= has length %zd",
+                       "ARRAY= has length %zd",
           boundary->ElementBytes(), elementLen);
     }
     if (boundaryRank > 0) {
@@ -547,7 +547,7 @@ void RTDEF(Eoshift)(Descriptor &result, const Descriptor &source,
         if (j != dim - 1) {
           if (boundary->GetDimension(k).Extent() != extent[j]) {
             terminator.Crash("EOSHIFT: BOUNDARY= has extent %jd on dimension "
-                             "%d but must conform with extent %jd of SOURCE=",
+                             "%d but must conform with extent %jd of ARRAY=",
                 static_cast<std::intmax_t>(boundary->GetDimension(k).Extent()),
                 k + 1, static_cast<std::intmax_t>(extent[j]));
           }
@@ -611,7 +611,7 @@ void RTDEF(EoshiftVector)(Descriptor &result, const Descriptor &source,
     RUNTIME_CHECK(terminator, boundary->type() == source.type());
     if (boundary->ElementBytes() != elementLen) {
       terminator.Crash("EOSHIFT: BOUNDARY= has element byte length %zd but "
-                       "SOURCE= has length %zd",
+                       "ARRAY= has length %zd",
           boundary->ElementBytes(), elementLen);
     }
   }
@@ -658,7 +658,7 @@ void RTDEF(Pack)(Descriptor &result, const Descriptor &source,
     RUNTIME_CHECK(terminator, vector->rank() == 1);
     RUNTIME_CHECK(terminator, source.type() == vector->type());
     if (source.ElementBytes() != vector->ElementBytes()) {
-      terminator.Crash("PACK: SOURCE= has element byte length %zd, but VECTOR= "
+      terminator.Crash("PACK: ARRAY= has element byte length %zd, but VECTOR= "
                        "has length %zd",
           source.ElementBytes(), vector->ElementBytes());
     }
diff --git a/flang/test/Evaluate/bug115923.f90 b/flang/test/Evaluate/bug115923.f90
new file mode 100644
index 00000000000000..c8cbaed1d254e9
--- /dev/null
+++ b/flang/test/Evaluate/bug115923.f90
@@ -0,0 +1,22 @@
+! RUN: %flang_fc1 -fsyntax-only -pedantic -Werror 2>&1 | FileCheck --allow-empty %s
+! Ensure that EOSHIFT's ARRAY= argument and result can be CLASS(*).
+! CHECK-NOT: error:
+! CHECK: warning: Source of TRANSFER is polymorphic
+! CHECK: warning: Mold of TRANSFER is polymorphic
+program p
+  type base
+    integer j
+  end type
+  type, extends(base) :: extended
+    integer k
+  end type
+  class(base), allocatable :: polyArray(:,:,:)
+  class(*), allocatable :: unlimited(:)
+  allocate(polyArray, source=reshape([(extended(n,n-1),n=1,8)],[2,2,2]))
+  allocate(unlimited, source=[(base(9),n=1,16)])
+  select type (x => eoshift(transfer(polyArray, unlimited), -4, base(-1)))
+    type is (base); print *, 'base', x
+    type is (extended); print *, 'extended?', x
+    class default; print *, 'class default??'
+  end select
+end

@klausler klausler force-pushed the bug115923 branch 3 times, most recently from 9327701 to 3404407 Compare November 14, 2024 01:09
Copy link
Contributor

@DanielCChen DanielCChen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick fix!
The changes LGTM. I have verified the original test cases pass with this patch.

The intrinsic processing code wasn't allowing the ARRAY= argument
to the EOSHIFT intrinsic function to be CLASS(*).  That case
seems to conform to the standard, although only one compiler could
actually handle it, so allow for it.

Fixes llvm#115923.
@klausler klausler merged commit 376713f into llvm:main Nov 14, 2024
5 of 7 checks passed
@klausler klausler deleted the bug115923 branch November 14, 2024 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:runtime flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Flang] Unlimited polymorphic ARRAY= argument is not supported by intrinsic EOSHIFT
3 participants