[Flang] Fix handling of unlimited polymorphic arrays

[ceseo]

Make sure that unlimited polymorphic arrays in WHERE constructs are processed through the runtime assignment path.

Fixes #133669

[llvmbot]

@llvm/pr-subscribers-flang-fir-hlfir

Author: Carlos Seo (ceseo)

Changes

Make sure that unlimited polymorphic arrays in WHERE constructs are processed through the runtime assignment path.

Fixes #133669

---

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

2 Files Affected:

• (modified) flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp (+2-1)
• (added) flang/test/HLFIR/unlimited-polymorphic-where-construct.f90 (+25)

diff --git a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
index 8104e53920c27..127dbd40f39d8 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
@@ -127,7 +127,8 @@ class AssignOpConversion : public mlir::OpRewritePattern<hlfir::AssignOp> {
                // types of the LHS and the RHS must match already. We use the
                // runtime in this case so that the polymorphic (including
                // unlimited) content is copied properly.
-               (lhs.isPolymorphic() && assignOp.isTemporaryLHS())) {
+               (lhs.isPolymorphic() && assignOp.isTemporaryLHS()) ||
+               lhs.isPolymorphic() || rhs.isPolymorphic()) {
       // Use the runtime for simplicity. An optimization pass will be added to
       // inline array assignment when profitable.
       mlir::Value from = emboxRHS(rhsExv);
diff --git a/flang/test/HLFIR/unlimited-polymorphic-where-construct.f90 b/flang/test/HLFIR/unlimited-polymorphic-where-construct.f90
new file mode 100644
index 0000000000000..8fdcfaca4e612
--- /dev/null
+++ b/flang/test/HLFIR/unlimited-polymorphic-where-construct.f90
@@ -0,0 +1,25 @@
+! RUN: flang -fc1 -emit-hlfir %s -o - | FileCheck %s
+
+module m1
+  type x
+  end type x
+  logical,parameter::t=.true.,f=.false.
+  logical::mask(3)=[t,f,t]
+end module m1
+
+subroutine s1
+  use m1
+  class(*),allocatable::v(:),u(:)
+  allocate(x::v(3))
+  allocate(x::u(3))
+  where(mask)
+     u=v
+  end where
+! CHECK: hlfir.region_assign
+! CHECK: !fir.ref<!fir.class<!fir.heap<!fir.array<?xnone>>>>
+end subroutine s1
+
+program main
+  call s1
+  print *,'pass'
+end program main

[jeanPerier]

Thanks @ceseo, while your fix makes some sense, I think this case is not well specified enough for us to implement.

What is the expected behavior o the following code? :

module m1
contains
subroutine s1(u,v)
  logical::mask(3)=[.true.,.false.,.true.]
  class(*),allocatable::v(:),u(:)
  where(mask)
     u=v
  end where
end subroutine s1
end module m1

program main
  use m1
  class(*),allocatable::v(:),u(:)
  v = [1.,2.,3.]
  u = [0, 0, 0]

  call s1(u,v)

  select type (u)
    type is (real)
      print *, "real", u
    type is (integer)
      print *, "integer", u
  end select
end program main

IFX and gfortran do not agree: https://godbolt.org/z/h14dsMP36

[jeanPerier]

Is this test testing the fix? Since the fix in the patch happens at the HLFIR to FIR level, I do not think -emit-hlfir exercises it.

[ceseo]

I think it is because if the polymorphic assignment was going through the scalar assignment path as before, we would see a fir.store instead of a hlfir.region_assign.

[jeanPerier]

Tricky case. Your fix implies that the LHS and RHS dynamic type is the same at runtime to work properly. I do not know what is the standard required behavior here....

In, assignment-stmt polymorphic LHS are required to be allocatable in F2023 10.2.1.2 (1) so that the LHS can be reallocated to the RHS dynamic type if it is not the same.

But here, inside a masked assignment, the reallocation logic is not really specified. Lowering is not generating it because the LHS/RHS shapes are supposed to match already.

I do think it would be cleaner to forbid assignments to whole polymorphic allocatables inside WHERE statements.

Section 10.2.3 does not really say anything about it allocatable assignements inside WHERE statements, and especially in the case of polymorphic LHS, this opens the doors to many interpretation/behaviors.

@klausler, what do you think? Should this case be clarified with the J3?

[klausler]

This case (assignment to polymorphic allocatable entity under one or more levels of WHERE or in DO CONCURRENT with a condition) can't change the type in a well-defined way, since the values of the elements that are masked out would become undefined. The behavior of other compilers doesn't give a clear portable precedent. I think that semantics should flag it as an error and we should document it in the section of Extensions.md about ambiguous and unclear semantics in the standard. Feel free to reassign this bug to me if you want. I don't advise asking J3 for an interpretation.

[ceseo]

This case (assignment to polymorphic allocatable entity under one or more levels of WHERE or in DO CONCURRENT with a condition) can't change the type in a well-defined way, since the values of the elements that are masked out would become undefined. The behavior of other compilers doesn't give a clear portable precedent. I think that semantics should flag it as an error and we should document it in the section of Extensions.md about ambiguous and unclear semantics in the standard. Feel free to reassign this bug to me if you want. I don't advise asking J3 for an interpretation.

Will do that, and close this PR then. Thanks!