[flang-commits] [flang] [flang][OpenMP] Make lastprivate work with reallocated variables (PR #106559)

[Leandro Lupori via flang-commits]

================
@@ -36,3 +24,45 @@ program lastprivate_allocatable
 ! CHECK-NEXT:            }
 ! CHECK-NEXT:            omp.yield
 ! CHECK-NEXT:          }
+program lastprivate_allocatable
+  integer, allocatable :: a
+  integer :: i
+  ! a is unallocated here
+  !$omp parallel do lastprivate(a)
+  do i=1,1
+    a = 42
+  enddo
+  !$omp end parallel do
+  ! a should be allocated here
+end program
+
+! CHECK-LABEL:  func @_QPlastprivate_realloc()
+! CHECK:          %[[A:.*]]:2 = hlfir.declare %{{.*}} {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "_QFlastprivate_reallocEa"} :
+! CHECK-SAME:       (!fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.complex<4>>>>>) ->
+! CHECK-SAME:       (!fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.complex<4>>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.complex<4>>>>>)
+! CHECK:          omp.parallel {
+! CHECK:            %[[A_PRIV:.*]]:2 = hlfir.declare %{{.*}} {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "_QFlastprivate_reallocEa"} :
+! CHECK-SAME:         (!fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.complex<4>>>>>) ->
+! CHECK-SAME:         (!fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.complex<4>>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.complex<4>>>>>)
+! CHECK:            omp.sections {
+! CHECK:              omp.section {
+! CHECK:                fir.if %{{.*}} {
----------------
luporl wrote:

This `if` was already being generated for allocatables in `copyVarHLFIR`, so I didn't want to change it.
It is indeed testing whether the original copy of the variable is allocated.
If I understood the standard correctly, it has this restriction:

> A variable that appears in a lastprivate clause must be definable.
(https://www.openmp.org/spec-html/5.2/openmpsu39.html)

The copy must also be allocated upon exit of the last section:

> If the original list item has the ALLOCATABLE attribute, the corresponding list item of which the value is assigned to the original item must have an allocation status of allocated upon exit from the sequentially last iteration or lexically last structured block sequence associated with a sections construct.

We don't check this, but since this is non-conforming code it should be OK. OTOH, following this logic, it would also be OK to not check if the original variable is allocated and let the behavior be undefined, at least for lastprivate.

> There is no such if statement in the lastprivate_allocatable testcase (the if statement is checking whether it is the final iteration of the loop).

Hmm, interesting, I hadn't noticed that. In fact, I think that `DataSharingProcessor::copyLastPrivateSymbol` could be almost entirely replaced by this new version of `copyHostAssociateVar/copyVarHLFIR`.

I don't know if, besides lastprivate, all other uses of `copyHostAssociateVar` can skip the `if allocated` part.
But if they do and If others agree we can remove it.

What do you think?

https://github.com/llvm/llvm-project/pull/106559