[flang][openmp] initialize allocatable components of firstprivate copies

[jeanPerier]

Descriptors of allocatable components of firstprivate derived type copies need to be set-up. Otherwise the program later die when manipulating them inside OpenMP region.

[llvmbot]

@llvm/pr-subscribers-flang-openmp

Author: None (jeanPerier)

Changes

Descriptors of allocatable components of firstprivate derived type copies need to be set-up. Otherwise the program later die when manipulating them inside OpenMP region.

---

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

2 Files Affected:

• (modified) flang/lib/Lower/Bridge.cpp (+5-1)
• (added) flang/test/Lower/OpenMP/firstprivate-alloc-comp.f90 (+19)

diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index c7bf4248155483..37f51d74d23f8f 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -833,7 +833,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
           if_builder.end();
         },
         [&](const auto &) -> void {
-          if (skipDefaultInit)
+          // Always initialize allocatable component descriptor, even when the
+          // value is later copied from the host (e.g. firstprivate) because the
+          // assignment from the host to the copy will fail if the component
+          // descriptors are not initialized.
+          if (skipDefaultInit && !hlfir::mayHaveAllocatableComponent(hSymType))
             return;
           // Initialize local/private derived types with default
           // initialization (Fortran 2023 section 11.1.7.5 and OpenMP 5.2
diff --git a/flang/test/Lower/OpenMP/firstprivate-alloc-comp.f90 b/flang/test/Lower/OpenMP/firstprivate-alloc-comp.f90
new file mode 100644
index 00000000000000..2453fe2c5208bc
--- /dev/null
+++ b/flang/test/Lower/OpenMP/firstprivate-alloc-comp.f90
@@ -0,0 +1,19 @@
+! Test delayed privatization for derived types with allocatable components.
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s | FileCheck %s
+
+subroutine firstprivate_alloc_comp
+  type t1
+    integer, allocatable :: c(:)
+  end type
+  type(t1) :: x
+  !$omp parallel firstprivate(x)
+    print *, allocated(x%c)
+  !$omp end parallel
+end
+
+  call firstprivate_alloc_comp()
+end
+! CHECK-LABEL:   omp.private {type = firstprivate} @_QFfirstprivate_alloc_compEx_firstprivate_ref_rec__QFfirstprivate_alloc_compTt1 : !fir.ref<!fir.type<_QFfirstprivate_alloc_compTt1{c:!fir.box<!fir.heap<!fir.array<?xi32>>>}>> alloc {
+! CHECK:     fir.call @_FortranAInitialize(
+! CHECK:   } copy {
+! ...

[llvmbot]

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

Author: None (jeanPerier)

Changes

Descriptors of allocatable components of firstprivate derived type copies need to be set-up. Otherwise the program later die when manipulating them inside OpenMP region.

---

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

2 Files Affected:

• (modified) flang/lib/Lower/Bridge.cpp (+5-1)
• (added) flang/test/Lower/OpenMP/firstprivate-alloc-comp.f90 (+19)

diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index c7bf4248155483..37f51d74d23f8f 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -833,7 +833,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
           if_builder.end();
         },
         [&](const auto &) -> void {
-          if (skipDefaultInit)
+          // Always initialize allocatable component descriptor, even when the
+          // value is later copied from the host (e.g. firstprivate) because the
+          // assignment from the host to the copy will fail if the component
+          // descriptors are not initialized.
+          if (skipDefaultInit && !hlfir::mayHaveAllocatableComponent(hSymType))
             return;
           // Initialize local/private derived types with default
           // initialization (Fortran 2023 section 11.1.7.5 and OpenMP 5.2
diff --git a/flang/test/Lower/OpenMP/firstprivate-alloc-comp.f90 b/flang/test/Lower/OpenMP/firstprivate-alloc-comp.f90
new file mode 100644
index 00000000000000..2453fe2c5208bc
--- /dev/null
+++ b/flang/test/Lower/OpenMP/firstprivate-alloc-comp.f90
@@ -0,0 +1,19 @@
+! Test delayed privatization for derived types with allocatable components.
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s | FileCheck %s
+
+subroutine firstprivate_alloc_comp
+  type t1
+    integer, allocatable :: c(:)
+  end type
+  type(t1) :: x
+  !$omp parallel firstprivate(x)
+    print *, allocated(x%c)
+  !$omp end parallel
+end
+
+  call firstprivate_alloc_comp()
+end
+! CHECK-LABEL:   omp.private {type = firstprivate} @_QFfirstprivate_alloc_compEx_firstprivate_ref_rec__QFfirstprivate_alloc_compTt1 : !fir.ref<!fir.type<_QFfirstprivate_alloc_compTt1{c:!fir.box<!fir.heap<!fir.array<?xi32>>>}>> alloc {
+! CHECK:     fir.call @_FortranAInitialize(
+! CHECK:   } copy {
+! ...

[tblah]

LGTM thanks!

[luporl]

LGTM.

I assumed the later copy would also handle the initialization of allocatable components. It's good to know that is not the case.

[jeanPerier]

I assumed the later copy would also handle the initialization of allocatable components. It's good to know that is not the case.

Right, the copy is made with an hlfir.assign, which lowers to a Assign runtime call currently because of the allocatable components. FortranAssign requires the descriptor to be properly initialized because it must preserve the LHS component allocations if they match the RHS component allocations (it needs to read the LHS component descriptor before doing anything with them, so they must be set to a valid state).

If AssignTemporary was used instead, your assumption would be correct because the LHS would be initialized before calling LHS.
Since the createHostAssociateVarClone code is not specific to OpenMP, I'd rather be safe and do the initialization explicitly here to cover all future usage of this helper, although a solution for OpenMP firstprivate could also have been to change the hlfir.assign in the copy region to an hlfir.assign temporary_lhs.