[flang-commits] [flang] [flang][OpenMP] Don't emit a privatization barrier for TASKLOOP (PR #228826)

[via flang-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-openmp

Author: Aditya Trivedi (adit4443ya)

<details>
<summary>Changes</summary>

Task privates are initialized by the encountering thread alone, so the barrier is unnecessary and deadlocks when one thread runs the taskloop (e.g. inside MASTER or SINGLE).

Fixes #<!-- -->186286

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


4 Files Affected:

- (modified) flang/lib/Lower/OpenMP/DataSharingProcessor.cpp (+7-2) 
- (modified) flang/lib/Lower/OpenMP/DataSharingProcessor.h (+2-1) 
- (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+1-1) 
- (added) flang/test/Lower/OpenMP/taskloop-lastprivate-barrier.f90 (+69) 


``````````diff
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 8d0d191058cfb..77c37ec2d84df 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -123,7 +123,7 @@ void DataSharingProcessor::processStep1(
 
   privatize(clauseOps, dir);
 
-  insertBarrier(clauseOps);
+  insertBarrier(clauseOps, dir);
 }
 
 void DataSharingProcessor::processStep2(mlir::Operation *op, bool isLoop) {
@@ -382,7 +382,12 @@ bool DataSharingProcessor::needBarrier() {
 }
 
 void DataSharingProcessor::insertBarrier(
-    mlir::omp::PrivateClauseOps *clauseOps) {
+    mlir::omp::PrivateClauseOps *clauseOps,
+    std::optional<llvm::omp::Directive> dir) {
+  // Task privates are set up by the encountering thread only; no barrier.
+  if (dir && llvm::omp::taskGeneratingSet.test(*dir))
+    return;
+
   if (!needBarrier())
     return;
 
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.h b/flang/lib/Lower/OpenMP/DataSharingProcessor.h
index 25d848c8baffc..d0e2f1116e014 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.h
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.h
@@ -150,7 +150,8 @@ class DataSharingProcessor {
       const omp::ObjectList &objects,
       llvm::SetVector<const semantics::Symbol *> &symbolSet);
   void collectSymbolsForPrivatization();
-  void insertBarrier(mlir::omp::PrivateClauseOps *clauseOps);
+  void insertBarrier(mlir::omp::PrivateClauseOps *clauseOps,
+                     std::optional<llvm::omp::Directive> dir);
   void collectDefaultSymbols();
   void collectImplicitSymbols();
   void collectPreDeterminedSymbols();
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 99a7882a658c8..3534b202c4c91 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -5307,7 +5307,7 @@ static mlir::omp::TaskloopContextOp genStandaloneTaskloop(
   DataSharingProcessor dsp(converter, semaCtx, item->clauses, eval,
                            /*shouldCollectPreDeterminedSymbols=*/true,
                            enableDelayedPrivatization, symTable);
-  dsp.processStep1(&taskloopClauseOps);
+  dsp.processStep1(&taskloopClauseOps, llvm::omp::Directive::OMPD_taskloop);
 
   if (hasPrivatizedArrayElementReduction(inReductionObjects,
                                          dsp.getAllSymbolsToPrivatize()))
diff --git a/flang/test/Lower/OpenMP/taskloop-lastprivate-barrier.f90 b/flang/test/Lower/OpenMP/taskloop-lastprivate-barrier.f90
new file mode 100644
index 0000000000000..61e12ad247234
--- /dev/null
+++ b/flang/test/Lower/OpenMP/taskloop-lastprivate-barrier.f90
@@ -0,0 +1,69 @@
+! TASKLOOP must not request a privatization barrier; worksharing loops still do.
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
+! RUN: bbc -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
+
+! Allocatable privatizer reads the original (mold).
+! CHECK-LABEL: func.func @_QPtaskloop_lastprivate_allocatable
+! CHECK:         omp.taskloop.context private(@{{.*}}Ea_private_box_heap_Uxi32
+! CHECK-NOT:     private_barrier
+! CHECK-SAME:    {
+! CHECK-NOT:     omp.barrier
+! CHECK:         omp.taskloop.wrapper
+subroutine taskloop_lastprivate_allocatable()
+  integer, allocatable :: a(:)
+  integer :: i
+  allocate(a(100))
+  a = -1
+  !$omp taskloop lastprivate(a)
+  do i = 1, 1
+    a(i) = i
+  end do
+  !$omp end taskloop
+end subroutine
+
+! Same variable firstprivate and lastprivate.
+! CHECK-LABEL: func.func @_QPtaskloop_first_and_lastprivate
+! CHECK:         omp.taskloop.context private(@{{.*}}Ex_firstprivate_i32
+! CHECK-NOT:     private_barrier
+! CHECK-SAME:    {
+! CHECK-NOT:     omp.barrier
+! CHECK:         omp.taskloop.wrapper
+subroutine taskloop_first_and_lastprivate()
+  integer :: x, i
+  x = 5
+  !$omp taskloop firstprivate(x) lastprivate(x)
+  do i = 1, 4
+    x = x + i
+  end do
+  !$omp end taskloop
+end subroutine
+
+! CHECK-LABEL: func.func @_QPtaskloop_nogroup_first_and_lastprivate
+! CHECK:         omp.taskloop.context nogroup private(@{{.*}}Ex_firstprivate_i32
+! CHECK-NOT:     private_barrier
+! CHECK-SAME:    {
+subroutine taskloop_nogroup_first_and_lastprivate()
+  integer :: x, i
+  x = 5
+  !$omp taskloop nogroup firstprivate(x) lastprivate(x)
+  do i = 1, 4
+    x = x + i
+  end do
+  !$omp end taskloop
+end subroutine
+
+! DO keeps the barrier.
+! CHECK-LABEL: func.func @_QPdo_first_and_lastprivate
+! CHECK:         omp.wsloop private(@{{.*}}Ex_firstprivate_i32 {{.*}}) private_barrier {
+subroutine do_first_and_lastprivate()
+  integer :: x, i
+  x = 5
+  !$omp parallel
+  !$omp do firstprivate(x) lastprivate(x)
+  do i = 1, 4
+    x = x + i
+  end do
+  !$omp end do
+  !$omp end parallel
+end subroutine

``````````

</details>


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