[Mlir-commits] [mlir] [OpenMP] Fix sections construct emitting a barrier with a nowait clause. (PR #238269)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-openmp

Author: JRP-arm

<details>
<summary>Changes</summary>

Currently the OpenMP sections construct still emits an implicit barrier at the end of its region when a reduction clause is present, even if a nowait clause is present. For example:
```fortran
subroutine repro(sum)
  integer :: sum
  !$omp sections reduction(+:sum)
  !$omp section
  sum = sum + 1
  !$omp end sections nowait
end subroutine
```
Still emits a barrier call: 
```sh 
flang -fopenmp -S -emit-llvm repro.f90 -o repro.ll
rg 'call.*@<!-- -->__kmpc_barrier' repro.ll
```
Update tests, add regression test.
AI-Assisted-By: Codex (OpenAI)

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


4 Files Affected:

- (modified) mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp (+1-4) 
- (modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+1-1) 
- (modified) mlir/test/Dialect/OpenMP/memory-effects.mlir (-6) 
- (modified) mlir/test/Target/LLVMIR/openmp-reduction-sections.mlir (+32-1) 


``````````diff
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 1c869ad04bbcf6..5aa8eec1d8dc5b 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -3533,10 +3533,7 @@ void SectionsOp::getEffects(
         &effects) {
   // The presence of the nowait clause eliminates the implicit barrier at the
   // end of a sections construct
-  // TODO: Presence of a nowait clause does not currently eliminate an implicit
-  // barrier if the reduction clause is also present (when it should), so we
-  // keep the barrier.
-  if (!getNowait() || (!getReductionVarsMutable().empty() && getNowait())) {
+  if (!getNowait()) {
     getBarrierEffects(effects);
   }
 
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index e8bcc22464439b..6f59a1fa457a79 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -1812,7 +1812,7 @@ static LogicalResult createReductionsAndCleanup(
     return op->emitOpError() << "failed to convert reductions";
 
   llvm::OpenMPIRBuilder::InsertPointTy afterIP = *contInsertPoint;
-  if (!isTeamsReduction) {
+  if (!isTeamsReduction && !isNowait) {
     llvm::OpenMPIRBuilder::InsertPointOrErrorTy barrierIP =
         ompBuilder->createBarrier({*contInsertPoint, reductionLoc},
                                   llvm::omp::OMPD_for);
diff --git a/mlir/test/Dialect/OpenMP/memory-effects.mlir b/mlir/test/Dialect/OpenMP/memory-effects.mlir
index b52f815c5e05cb..3325e00a6279be 100644
--- a/mlir/test/Dialect/OpenMP/memory-effects.mlir
+++ b/mlir/test/Dialect/OpenMP/memory-effects.mlir
@@ -94,11 +94,7 @@ combiner {
 }
 
 func.func @sections_bounded_reduction_effects(%x: memref<f32>) {
-  // TODO: nowait does not currently eliminate the barrier with reductions.
-  // The first two effects come from the implicit barrier.
   // expected-remark at below {{recursive memory effects}}
-  // expected-remark at below {{found an instance of 'read' on resource '<Default>'}}
-  // expected-remark at below {{found an instance of 'write' on resource '<Default>'}}
   // expected-remark at below {{found an instance of 'allocate' on block argument 0, on resource '<Default>'}}
   // expected-remark at below {{found an instance of 'free' on block argument 0, on resource '<Default>'}}
   // expected-remark at below {{found an instance of 'read' on block argument 0, on resource '<Default>'}}
@@ -155,8 +151,6 @@ func.func @sections_unbounded_private_bounded_reduction(%x: memref<i32>, %y: mem
 func.func @sections_bounded_private_bounded_reduction(%x: memref<i32>, %y: memref<f32>) {
   // The private and reduction clauses make independent assumptions.
   // expected-remark at below {{recursive memory effects}}
-  // expected-remark at below {{found an instance of 'read' on resource '<Default>'}}
-  // expected-remark at below {{found an instance of 'write' on resource '<Default>'}}
   // expected-remark at below {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
   // expected-remark at below {{found an instance of 'allocate' on block argument 0, on resource '<Default>'}}
   // expected-remark at below {{found an instance of 'write' on block argument 0, on resource '<Default>'}}
diff --git a/mlir/test/Target/LLVMIR/openmp-reduction-sections.mlir b/mlir/test/Target/LLVMIR/openmp-reduction-sections.mlir
index c8394144708a1f..2a088327d845c4 100644
--- a/mlir/test/Target/LLVMIR/openmp-reduction-sections.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-reduction-sections.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+// RUN: mlir-translate -mlir-to-llvmir -split-input-file %s | FileCheck %s
 
 omp.declare_reduction @add_reduction_f32 : f32 init {
 ^bb0(%arg0: f32):
@@ -157,3 +157,34 @@ llvm.func @sections_(%arg0: !llvm.ptr {fir.bindc_name = "x"}) attributes {fir.in
 // CHECK:         %[[VAL_78:.*]] = fadd contract float %[[VAL_73]], %[[VAL_77]]
 // CHECK:         store float %[[VAL_78]], ptr %[[VAL_72]], align 4
 // CHECK:         ret void
+
+// -----
+
+omp.declare_reduction @add_reduction_f32 : f32 init {
+^bb0(%arg0: f32):
+  %0 = llvm.mlir.constant(0.0 : f32) : f32
+  omp.yield(%0 : f32)
+} combiner {
+^bb0(%arg0: f32, %arg1: f32):
+  %0 = llvm.fadd %arg0, %arg1 : f32
+  omp.yield(%0 : f32)
+}
+
+llvm.func @sections_nowait(%sum: !llvm.ptr) {
+  omp.sections nowait reduction(@add_reduction_f32 %sum -> %private : !llvm.ptr) {
+    omp.section {
+    ^bb0(%value: !llvm.ptr):
+      omp.terminator
+    }
+    omp.terminator
+  }
+  llvm.return
+}
+
+// CHECK-LABEL: define void @sections_nowait(
+// CHECK-NOT:   @__kmpc_barrier
+// CHECK:       call i32 @__kmpc_reduce_nowait(
+// CHECK-NOT:   @__kmpc_barrier
+// CHECK:       call void @__kmpc_end_reduce_nowait(
+// CHECK-NOT:   @__kmpc_barrier
+// CHECK:       {{^}}}

``````````

</details>


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