[flang][OpenMP] Handle omp.private in FirOpBuilder::getAllocaBlock()

[ergawy]

Fixes a crash uncovered by pr89651 in the test suite.

Fixes a crash caused by missing handling of omp.private ops in FirOpBuilder::getAllocaBlock().

[llvmbot]

@llvm/pr-subscribers-mlir-openacc
@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-openacc
@llvm/pr-subscribers-flang-openmp

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

Author: Kareem Ergawy (ergawy)

Changes

Fixes a crash uncovered by pr89651 in the test suite.

Fixes a crash caused by missing handling of omp.private ops in FirOpBuilder::getAllocaBlock().

---

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

2 Files Affected:

• (modified) flang/lib/Optimizer/Builder/FIRBuilder.cpp (+2-1)
• (modified) flang/test/Lower/OpenMP/delayed-privatization-allocatable-firstprivate.f90 (+19-2)

diff --git a/flang/lib/Optimizer/Builder/FIRBuilder.cpp b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
index 3c3fd02d7c88e..48d4397f448e8 100644
--- a/flang/lib/Optimizer/Builder/FIRBuilder.cpp
+++ b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
@@ -252,7 +252,8 @@ mlir::Block *fir::FirOpBuilder::getAllocaBlock() {
               .getParentOfType<mlir::omp::OutlineableOpenMPOpInterface>()) {
     return ompOutlineableIface.getAllocaBlock();
   }
-  if (getRegion().getParentOfType<mlir::omp::DeclareReductionOp>())
+  if (getRegion().getParentOfType<mlir::omp::DeclareReductionOp>() ||
+      getRegion().getParentOfType<mlir::omp::PrivateClauseOp>())
     return &getRegion().front();
   if (auto accRecipeIface =
           getRegion().getParentOfType<mlir::acc::RecipeInterface>()) {
diff --git a/flang/test/Lower/OpenMP/delayed-privatization-allocatable-firstprivate.f90 b/flang/test/Lower/OpenMP/delayed-privatization-allocatable-firstprivate.f90
index aa835f82b2730..8a324b77226d7 100644
--- a/flang/test/Lower/OpenMP/delayed-privatization-allocatable-firstprivate.f90
+++ b/flang/test/Lower/OpenMP/delayed-privatization-allocatable-firstprivate.f90
@@ -1,10 +1,13 @@
 ! Test delayed privatization for allocatables: `firstprivate`.
 
+! RUN: split-file %s %t
+
 ! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
-! RUN:   -o - %s 2>&1 | FileCheck %s
-! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %s 2>&1 |\
+! RUN:   -o - %t/test_ir.f90 2>&1 | FileCheck %s
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %t/test_ir.f90 2>&1 |\
 ! RUN:   FileCheck %s
 
+!--- test_ir.f90
 subroutine delayed_privatization_allocatable
   implicit none
   integer, allocatable :: var1
@@ -34,3 +37,17 @@ subroutine delayed_privatization_allocatable
 ! CHECK-NEXT:    %[[ORIG_BASE_LD:.*]] = fir.load %[[ORIG_BASE_ADDR]]
 ! CHECK-NEXT:    hlfir.assign %[[ORIG_BASE_LD]] to %[[PRIV_BASE_BOX]] temporary_lhs
 ! CHECK-NEXT:  }
+
+! RUN: %flang -fopenmp -mmlir --openmp-enable-delayed-privatization \
+! RUN:   -o - %t/test_compilation_to_obj.f90
+
+!--- test_compilation_to_obj.f90
+
+program compilation_to_obj
+  real, allocatable :: t(:)
+
+!$omp parallel firstprivate(t)
+  t(1) = 3.14
+!$omp end parallel
+
+end program compilation_to_obj

[tblah]

Code change LGTM. Thanks for the bug fix

[clementval]

adding a common interface to these kind of ops would be much cleaner and future proof.

[ergawy]

Makes sense. I updated the PR to reuse the RecipeInterface for both OpenMP and OpenACC.