[Mlir-commits] [flang] [mlir] [mlir][OpenMP] Model memory effects of omp.section and omp.sections (PR #230167)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Fri Oct 9 02:13:39 PDT 2026
https://github.com/JRP-arm updated https://github.com/llvm/llvm-project/pull/230167
>From 369d67c9ed18f9220c3c3068cdd20fe360c7465e Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com"
<jamrow01 at ip-10-252-16-38.eu-west-1.compute.internal>
Date: Fri, 2 Oct 2026 10:16:31 +0000
Subject: [PATCH 1/3] [mlir][OpenMP] Model memory effects of omp.section and
omp.sections
Model recursive effects of section bodies and account for implicit barriers,
private and lastprivate variables, and reduction recipes in sections.
Add shared helpers for symbol-referenced privatisation recipes, test memory
effects and canonicalisation, and update the Flang sections lowering tests.
---
flang/test/Lower/OpenMP/sections.f90 | 5 +-
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 6 +-
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 94 +++++++++++++++++++
mlir/test/Dialect/OpenMP/canonicalize.mlir | 80 ++++++++++++++++
mlir/test/Dialect/OpenMP/memory-effects.mlir | 78 +++++++++++++++
5 files changed, 258 insertions(+), 5 deletions(-)
create mode 100644 mlir/test/Dialect/OpenMP/memory-effects.mlir
diff --git a/flang/test/Lower/OpenMP/sections.f90 b/flang/test/Lower/OpenMP/sections.f90
index f1e089caa81017..10668c291996ff 100644
--- a/flang/test/Lower/OpenMP/sections.f90
+++ b/flang/test/Lower/OpenMP/sections.f90
@@ -56,6 +56,7 @@
!CHECK: omp.terminator
!CHECK: }
!CHECK: omp.sections nowait {
+!CHECK: fir.call @_QPfoo() fastmath<contract> : () -> ()
!CHECK: omp.terminator
!CHECK: }
!CHECK: return
@@ -77,6 +78,7 @@ program sample
!$omp end sections
!$omp sections
+ call foo()
!$omp end sections nowait
end program sample
@@ -87,9 +89,6 @@ end program sample
!CHECK: %[[TEMP:.*]] = fir.load %[[ARG_DECL]]#0 : !fir.ref<f32>
!CHECK: hlfir.assign %[[TEMP]] to %[[PRIVATE_ALPHA_DECL]]#0 : f32, !fir.ref<f32>
!CHECK: omp.sections {
-!CHECK: omp.section {
-!CHECK: omp.terminator
-!CHECK: }
!CHECK: omp.terminator
!CHECK: }
!CHECK: omp.sections {
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 6bc99cf7656992..8c0780c23c13ff 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -276,7 +276,8 @@ def TeamsOp : OpenMP_Op<"teams", traits = [
//===----------------------------------------------------------------------===//
def SectionOp : OpenMP_Op<"section", traits = [
- BlockArgOpenMPOpInterface, HasParent<"SectionsOp">
+ BlockArgOpenMPOpInterface, HasParent<"SectionsOp">,
+ RecursiveMemoryEffects
], singleRegion = true> {
let summary = "section directive";
let description = [{
@@ -298,7 +299,8 @@ def SectionOp : OpenMP_Op<"section", traits = [
}
def SectionsOp : OpenMP_Op<"sections", traits = [
- AttrSizedOperandSegments, DeclareOpInterfaceMethods<ComposableOpInterface>
+ AttrSizedOperandSegments, DeclareOpInterfaceMethods<ComposableOpInterface>,
+ DeclareOpInterfaceMethods<MemoryEffectsOpInterface>, RecursiveMemoryEffects
], clauses = [
OpenMP_AllocateClause, OpenMP_NowaitClause, OpenMP_PrivateClause,
OpenMP_ReductionClause
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index c6a424bd932146..0b70d8d75da193 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -3258,6 +3258,100 @@ LogicalResult SectionsOp::verifyRegions() {
return success();
}
+/// Return whether all operations in the op's regions are memory-effect-free.
+static bool areRegionsMemoryEffectFree(Operation *op) {
+ for (Region ®ion : op->getRegions())
+ for (Operation &nestedOp : region.getOps())
+ if (!isMemoryEffectFree(&nestedOp))
+ return false;
+ return true;
+}
+
+/// Model private storage and the effects of referenced privatisation recipes.
+template <typename OpType>
+static void getPrivatisationEffects(
+ OpType op, SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
+ std::optional<ArrayAttr> privateSyms = op.getPrivateSyms();
+ if (!privateSyms)
+ return;
+
+ MutableOperandRange privateVars = op.getPrivateVarsMutable();
+ auto iface = cast<BlockArgOpenMPOpInterface>(op.getOperation());
+ Block::BlockArgListType privateArgs = iface.getPrivateBlockArgs();
+ for (auto entry : llvm::zip_equal(privateVars, privateArgs, *privateSyms)) {
+ OpOperand &original = std::get<0>(entry);
+ BlockArgument privateCopy = std::get<1>(entry);
+
+ effects.emplace_back(MemoryEffects::Allocate::get(), privateCopy);
+ effects.emplace_back(MemoryEffects::Free::get(), privateCopy);
+ // private variables may read the original as a mold
+ effects.emplace_back(MemoryEffects::Read::get(), &original);
+
+ auto privateSym = cast<SymbolRefAttr>(std::get<2>(entry));
+ auto privatiser =
+ SymbolTable::lookupNearestSymbolFrom<PrivateClauseOp>(op, privateSym);
+ // RecursiveMemoryEffects only covers regions nested in the directive, not
+ // symbol-referenced regions, user defined types have arbitrary recipe bodies.
+ // Default to a conservative model of read and write to arbitrary memory.
+ if (!privatiser || !areRegionsMemoryEffectFree(privatiser)) {
+ effects.emplace_back(MemoryEffects::Read::get());
+ effects.emplace_back(MemoryEffects::Write::get());
+ }
+ }
+}
+
+void SectionsOp::getEffects(
+ llvm::SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>>
+ &effects) {
+ // TODO: Nowait does not eliminate an implicit barrier if the reduction clause
+ // is also present
+ if (!getNowait() || (!getReductionVarsMutable().empty() && getNowait())) {
+ effects.emplace_back(MemoryEffects::Read::get());
+ effects.emplace_back(MemoryEffects::Write::get());
+ }
+
+ getPrivatisationEffects(*this, effects);
+ auto iface = cast<BlockArgOpenMPOpInterface>(getOperation());
+ // We cannot determine if the variable is lastprivate, so assume all privates
+ // are.
+ for (auto entry :
+ llvm::zip_equal(getPrivateVarsMutable(), iface.getPrivateBlockArgs())) {
+ effects.emplace_back(MemoryEffects::Read::get(), std::get<1>(entry));
+ effects.emplace_back(MemoryEffects::Write::get(), &std::get<0>(entry));
+ }
+
+ Block::BlockArgListType reductionArgs = iface.getReductionBlockArgs();
+ MutableOperandRange reductionVars = getReductionVarsMutable();
+
+ for (auto entry : llvm::zip_equal(reductionVars, reductionArgs)) {
+ OpOperand &original = std::get<0>(entry);
+ BlockArgument privateAccumulator = std::get<1>(entry);
+
+ effects.emplace_back(MemoryEffects::Allocate::get(), privateAccumulator);
+ effects.emplace_back(MemoryEffects::Free::get(), privateAccumulator);
+
+ effects.emplace_back(MemoryEffects::Read::get(), privateAccumulator);
+
+ effects.emplace_back(MemoryEffects::Read::get(), &original);
+ effects.emplace_back(MemoryEffects::Write::get(), &original);
+ }
+
+ if (auto reductionSyms = getReductionSyms()) {
+ for (Attribute attr : *reductionSyms) {
+ auto recipe = SymbolTable::lookupNearestSymbolFrom<DeclareReductionOp>(
+ *this, cast<SymbolRefAttr>(attr));
+ if (recipe && areRegionsMemoryEffectFree(recipe))
+ continue;
+ // RecursiveMemoryEffects only covers operations nested in this directive.
+ // Reduction recipes are separate declarations referenced by symbol, so
+ // their effects must be accounted for explicitly.
+ effects.emplace_back(MemoryEffects::Read::get());
+ effects.emplace_back(MemoryEffects::Write::get());
+ break;
+ }
+ }
+}
+
//===----------------------------------------------------------------------===//
// ScopeOp
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/OpenMP/canonicalize.mlir b/mlir/test/Dialect/OpenMP/canonicalize.mlir
index cc42f2ed8618c7..4b65b0d648f048 100644
--- a/mlir/test/Dialect/OpenMP/canonicalize.mlir
+++ b/mlir/test/Dialect/OpenMP/canonicalize.mlir
@@ -143,3 +143,83 @@ func.func @constant_hoisting_target(%x : !llvm.ptr) {
// CHECK-NOT: arith.constant
// CHECK: omp.target
// CHECK: arith.constant
+
+// -----
+
+// Pure sections and their enclosing nowait construct can both be removed.
+// CHECK-LABEL: func.func @sections_nowait_no_side_effects
+// CHECK-NEXT: return
+func.func @sections_nowait_no_side_effects(%a: i32, %b: i32) {
+ omp.sections nowait {
+ omp.section {
+ %unused = arith.addi %a, %b : i32
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
+
+// -----
+
+// Sections also retain their implicit barrier when their body is empty.
+// CHECK-LABEL: func.func @sections_only_terminator
+// CHECK-NEXT: omp.sections {
+// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: }
+// CHECK-NEXT: return
+func.func @sections_only_terminator() {
+ omp.sections {
+ omp.terminator
+ }
+ return
+}
+
+// -----
+
+// Effects must be considered recursively through both sections and section.
+// CHECK-LABEL: func.func @sections_nowait_store
+// CHECK-SAME: (%[[X:.*]]: memref<i32>, %[[VALUE:.*]]: i32)
+// CHECK-NEXT: omp.sections nowait {
+// CHECK-NEXT: omp.section {
+// CHECK-NEXT: memref.store %[[VALUE]], %[[X]][] : memref<i32>
+// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: }
+// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: }
+// CHECK-NEXT: return
+func.func @sections_nowait_store(%x: memref<i32>, %value: i32) {
+ omp.sections nowait {
+ omp.section {
+ memref.store %value, %x[] : memref<i32>
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
+
+// -----
+
+// Unknown effects in a nested operation also prevent removal.
+// CHECK-LABEL: func.func @sections_nowait_call
+// CHECK-NEXT: omp.sections nowait {
+// CHECK-NEXT: omp.section {
+// CHECK-NEXT: func.call @opaque() : () -> ()
+// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: }
+// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: }
+// CHECK-NEXT: return
+func.func @sections_nowait_call() {
+ omp.sections nowait {
+ omp.section {
+ func.call @opaque() : () -> ()
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
+
+func.func private @opaque()
diff --git a/mlir/test/Dialect/OpenMP/memory-effects.mlir b/mlir/test/Dialect/OpenMP/memory-effects.mlir
new file mode 100644
index 00000000000000..06dd2d7f493440
--- /dev/null
+++ b/mlir/test/Dialect/OpenMP/memory-effects.mlir
@@ -0,0 +1,78 @@
+// RUN: mlir-opt %s --test-side-effects --verify-diagnostics
+
+func.func @sections_effects() {
+ // An implicit barrier reads and writes unspecified memory.
+ // expected-remark at +2 {{found an instance of 'read' on resource '<Default>'}}
+ // expected-remark at +1 {{found an instance of 'write' on resource '<Default>'}}
+ omp.sections {
+ omp.section {
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
+
+func.func @sections_nowait_no_effects() {
+ // expected-remark at +1 {{operation has no memory effects}}
+ omp.sections nowait {
+ omp.section {
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
+
+omp.private {type = private} @sections_p : memref<i32>
+
+func.func @sections_private_effects(%x: memref<i32>) {
+ // expected-remark at +5 {{found an instance of 'allocate' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +4 {{found an instance of 'free' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +3 {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
+ // expected-remark at +2 {{found an instance of 'read' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +1 {{found an instance of 'write' on op operand 0, on resource '<Default>'}}
+ omp.sections nowait private(@sections_p %x -> %private_x : memref<i32>) {
+ omp.section {
+ ^bb0(%section_x: memref<i32>):
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
+
+omp.declare_reduction @add_f32 : f32
+init {
+^bb0(%unused: f32):
+ // expected-remark at +1 {{operation has no memory effects}}
+ %zero = arith.constant 0.0 : f32
+ omp.yield(%zero : f32)
+}
+combiner {
+^bb0(%lhs: f32, %rhs: f32):
+ // expected-remark at +1 {{operation has no memory effects}}
+ %sum = arith.addf %lhs, %rhs : f32
+ omp.yield(%sum : f32)
+}
+
+func.func @sections_reduction_effects(%x: memref<f32>) {
+ // TODO: Nowait does not eliminate an implicit barrier if the reduction clause is also present,
+ // requiring the below two remarks
+ // expected-remark at +7 {{found an instance of 'read' on resource '<Default>'}}
+ // expected-remark at +6 {{found an instance of 'write' on resource '<Default>'}}
+ // expected-remark at +5 {{found an instance of 'allocate' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +4 {{found an instance of 'free' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +3 {{found an instance of 'read' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +2 {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
+ // expected-remark at +1 {{found an instance of 'write' on op operand 0, on resource '<Default>'}}
+ omp.sections nowait reduction(
+ @add_f32 %x -> %private_x : memref<f32>) {
+ omp.section {
+ ^bb0(%section_x: memref<f32>):
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
>From 0e9bb2e56dbe74f9d8e9afe89095aaa6bb37e8a3 Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com"
<jamrow01 at ip-10-252-16-38.eu-west-1.compute.internal>
Date: Fri, 2 Oct 2026 10:16:31 +0000
Subject: [PATCH 2/3] [mlir][OpenMP] Model memory effects of omp.section and
omp.sections
Model recursive effects of section bodies and account for implicit barriers,
private and lastprivate variables, and reduction recipes in sections.
Add shared helpers for symbol-referenced privatisation recipes, test memory
effects and canonicalisation, and update the Flang sections lowering tests.
---
flang/test/Lower/OpenMP/sections.f90 | 5 +-
.../mlir/Dialect/OpenMP/OpenMPDialect.h | 9 ++
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 6 +-
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 124 ++++++++++++++++++
mlir/test/Dialect/OpenMP/canonicalize.mlir | 80 +++++++++++
mlir/test/Dialect/OpenMP/memory-effects.mlir | 78 +++++++++++
6 files changed, 297 insertions(+), 5 deletions(-)
create mode 100644 mlir/test/Dialect/OpenMP/memory-effects.mlir
diff --git a/flang/test/Lower/OpenMP/sections.f90 b/flang/test/Lower/OpenMP/sections.f90
index f1e089caa81017..10668c291996ff 100644
--- a/flang/test/Lower/OpenMP/sections.f90
+++ b/flang/test/Lower/OpenMP/sections.f90
@@ -56,6 +56,7 @@
!CHECK: omp.terminator
!CHECK: }
!CHECK: omp.sections nowait {
+!CHECK: fir.call @_QPfoo() fastmath<contract> : () -> ()
!CHECK: omp.terminator
!CHECK: }
!CHECK: return
@@ -77,6 +78,7 @@ program sample
!$omp end sections
!$omp sections
+ call foo()
!$omp end sections nowait
end program sample
@@ -87,9 +89,6 @@ end program sample
!CHECK: %[[TEMP:.*]] = fir.load %[[ARG_DECL]]#0 : !fir.ref<f32>
!CHECK: hlfir.assign %[[TEMP]] to %[[PRIVATE_ALPHA_DECL]]#0 : f32, !fir.ref<f32>
!CHECK: omp.sections {
-!CHECK: omp.section {
-!CHECK: omp.terminator
-!CHECK: }
!CHECK: omp.terminator
!CHECK: }
!CHECK: omp.sections {
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h b/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
index a6854752f75b66..0af6515309ad3c 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
@@ -61,6 +61,15 @@ std::tuple<NewCliOp, OpOperand *, OpOperand *> decodeCli(mlir::Value cli);
/// OpenMP dialect enum. The LLVM 'default' and 'unknown' kinds have no dialect
/// counterpart and are not valid inputs.
ClauseProcBindKind convertProcBindKind(llvm::omp::ProcBindKind kind);
+
+/// Models shared OpenMP runtime bookkeeping used to manage threadprivate
+/// variables including caches.
+struct ThreadprivateRuntimeResource
+ : SideEffects::Resource::Base<ThreadprivateRuntimeResource> {
+ StringRef getName() const final { return "OpenMPThreadprivateRuntime"; }
+
+ bool isAddressable() const override { return false; }
+};
} // namespace mlir::omp
#endif // MLIR_DIALECT_OPENMP_OPENMPDIALECT_H_
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 6bc99cf7656992..8c0780c23c13ff 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -276,7 +276,8 @@ def TeamsOp : OpenMP_Op<"teams", traits = [
//===----------------------------------------------------------------------===//
def SectionOp : OpenMP_Op<"section", traits = [
- BlockArgOpenMPOpInterface, HasParent<"SectionsOp">
+ BlockArgOpenMPOpInterface, HasParent<"SectionsOp">,
+ RecursiveMemoryEffects
], singleRegion = true> {
let summary = "section directive";
let description = [{
@@ -298,7 +299,8 @@ def SectionOp : OpenMP_Op<"section", traits = [
}
def SectionsOp : OpenMP_Op<"sections", traits = [
- AttrSizedOperandSegments, DeclareOpInterfaceMethods<ComposableOpInterface>
+ AttrSizedOperandSegments, DeclareOpInterfaceMethods<ComposableOpInterface>,
+ DeclareOpInterfaceMethods<MemoryEffectsOpInterface>, RecursiveMemoryEffects
], clauses = [
OpenMP_AllocateClause, OpenMP_NowaitClause, OpenMP_PrivateClause,
OpenMP_ReductionClause
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index c6a424bd932146..89c830dc07b329 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -3258,6 +3258,130 @@ LogicalResult SectionsOp::verifyRegions() {
return success();
}
+/// Return whether all operations in the op's regions are memory-effect-free.
+static bool areRegionsMemoryEffectFree(Operation *op) {
+ for (Region ®ion : op->getRegions())
+ for (Operation &nestedOp : region.getOps())
+ if (!isMemoryEffectFree(&nestedOp))
+ return false;
+ return true;
+}
+
+/// Model private storage and the effects of referenced privatisation recipes.
+template <typename OpType>
+static void getPrivatisationEffects(
+ OpType op, SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
+ std::optional<ArrayAttr> privateSyms = op.getPrivateSyms();
+ if (!privateSyms)
+ return;
+
+ MutableOperandRange privateVars = op.getPrivateVarsMutable();
+ auto iface = cast<BlockArgOpenMPOpInterface>(op.getOperation());
+ Block::BlockArgListType privateArgs = iface.getPrivateBlockArgs();
+ for (auto entry : llvm::zip_equal(privateVars, privateArgs, *privateSyms)) {
+ OpOperand &original = std::get<0>(entry);
+ BlockArgument privateCopy = std::get<1>(entry);
+
+ effects.emplace_back(MemoryEffects::Allocate::get(), privateCopy);
+ effects.emplace_back(MemoryEffects::Free::get(), privateCopy);
+ // private variables may read the original as a mold
+ effects.emplace_back(MemoryEffects::Read::get(), &original);
+
+ auto privateSym = cast<SymbolRefAttr>(std::get<2>(entry));
+ auto privatiser =
+ SymbolTable::lookupNearestSymbolFrom<PrivateClauseOp>(op, privateSym);
+ // RecursiveMemoryEffects only covers regions nested in the directive, not
+ // symbol-referenced regions, user defined types have arbitrary recipe
+ // bodies. Default to a conservative model of read and write to arbitrary
+ // memory.
+ if (!privatiser || !areRegionsMemoryEffectFree(privatiser)) {
+ effects.emplace_back(MemoryEffects::Read::get());
+ effects.emplace_back(MemoryEffects::Write::get());
+ }
+ }
+}
+
+template <typename OpType>
+static void
+getLastprivateEffects(OpType op,
+ SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
+ auto iface = cast<BlockArgOpenMPOpInterface>(op.getOperation());
+ // We cannot determine if the variable is lastprivate, so assume all private
+ // variables are.
+ for (auto entry : llvm::zip_equal(op.getPrivateVarsMutable(),
+ iface.getPrivateBlockArgs())) {
+ effects.emplace_back(MemoryEffects::Read::get(), std::get<1>(entry));
+ effects.emplace_back(MemoryEffects::Write::get(), &std::get<0>(entry));
+ }
+}
+
+/// Model accumulator initialisation and destruction and referenced recipes for
+/// a reduction clause.
+static void
+getReductionEffects(Operation *op, MutableOperandRange reductionVars,
+ ArrayRef<BlockArgument> reductionArgs,
+ std::optional<ArrayAttr> reductionSyms,
+ SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
+ for (auto entry : llvm::zip_equal(reductionVars, reductionArgs)) {
+ OpOperand &original = std::get<0>(entry);
+ BlockArgument privateAccumulator = std::get<1>(entry);
+
+ effects.emplace_back(MemoryEffects::Allocate::get(), privateAccumulator);
+ effects.emplace_back(MemoryEffects::Free::get(), privateAccumulator);
+
+ effects.emplace_back(MemoryEffects::Read::get(), privateAccumulator);
+ effects.emplace_back(MemoryEffects::Write::get(), privateAccumulator);
+
+ effects.emplace_back(MemoryEffects::Read::get(), &original);
+ effects.emplace_back(MemoryEffects::Write::get(), &original);
+ }
+
+ if (reductionSyms) {
+ for (Attribute attr : *reductionSyms) {
+ auto recipe = SymbolTable::lookupNearestSymbolFrom<DeclareReductionOp>(
+ op, cast<SymbolRefAttr>(attr));
+ if (recipe && areRegionsMemoryEffectFree(recipe))
+ continue;
+ // RecursiveMemoryEffects only covers operations nested in this directive.
+ // Reduction recipes are separate declarations referenced by symbol, so
+ // their effects must be accounted for explicitly.
+ effects.emplace_back(MemoryEffects::Read::get());
+ effects.emplace_back(MemoryEffects::Write::get());
+ break;
+ }
+ }
+}
+
+static void getBarrierEffects(
+ llvm::SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>>
+ &effects) {
+ effects.emplace_back(MemoryEffects::Read::get(),
+ ThreadprivateRuntimeResource::get());
+ effects.emplace_back(MemoryEffects::Write::get(),
+ ThreadprivateRuntimeResource::get());
+}
+
+void SectionsOp::getEffects(
+ llvm::SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>>
+ &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())) {
+ getBarrierEffects(effects);
+ }
+
+ getPrivatisationEffects(*this, effects);
+ getLastprivateEffects(*this, effects);
+
+ auto iface = cast<BlockArgOpenMPOpInterface>(getOperation());
+ getReductionEffects(*this, getReductionVarsMutable(),
+ iface.getReductionBlockArgs(), getReductionSyms(),
+ effects);
+}
+
//===----------------------------------------------------------------------===//
// ScopeOp
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/OpenMP/canonicalize.mlir b/mlir/test/Dialect/OpenMP/canonicalize.mlir
index cc42f2ed8618c7..4b65b0d648f048 100644
--- a/mlir/test/Dialect/OpenMP/canonicalize.mlir
+++ b/mlir/test/Dialect/OpenMP/canonicalize.mlir
@@ -143,3 +143,83 @@ func.func @constant_hoisting_target(%x : !llvm.ptr) {
// CHECK-NOT: arith.constant
// CHECK: omp.target
// CHECK: arith.constant
+
+// -----
+
+// Pure sections and their enclosing nowait construct can both be removed.
+// CHECK-LABEL: func.func @sections_nowait_no_side_effects
+// CHECK-NEXT: return
+func.func @sections_nowait_no_side_effects(%a: i32, %b: i32) {
+ omp.sections nowait {
+ omp.section {
+ %unused = arith.addi %a, %b : i32
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
+
+// -----
+
+// Sections also retain their implicit barrier when their body is empty.
+// CHECK-LABEL: func.func @sections_only_terminator
+// CHECK-NEXT: omp.sections {
+// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: }
+// CHECK-NEXT: return
+func.func @sections_only_terminator() {
+ omp.sections {
+ omp.terminator
+ }
+ return
+}
+
+// -----
+
+// Effects must be considered recursively through both sections and section.
+// CHECK-LABEL: func.func @sections_nowait_store
+// CHECK-SAME: (%[[X:.*]]: memref<i32>, %[[VALUE:.*]]: i32)
+// CHECK-NEXT: omp.sections nowait {
+// CHECK-NEXT: omp.section {
+// CHECK-NEXT: memref.store %[[VALUE]], %[[X]][] : memref<i32>
+// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: }
+// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: }
+// CHECK-NEXT: return
+func.func @sections_nowait_store(%x: memref<i32>, %value: i32) {
+ omp.sections nowait {
+ omp.section {
+ memref.store %value, %x[] : memref<i32>
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
+
+// -----
+
+// Unknown effects in a nested operation also prevent removal.
+// CHECK-LABEL: func.func @sections_nowait_call
+// CHECK-NEXT: omp.sections nowait {
+// CHECK-NEXT: omp.section {
+// CHECK-NEXT: func.call @opaque() : () -> ()
+// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: }
+// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: }
+// CHECK-NEXT: return
+func.func @sections_nowait_call() {
+ omp.sections nowait {
+ omp.section {
+ func.call @opaque() : () -> ()
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
+
+func.func private @opaque()
diff --git a/mlir/test/Dialect/OpenMP/memory-effects.mlir b/mlir/test/Dialect/OpenMP/memory-effects.mlir
new file mode 100644
index 00000000000000..06dd2d7f493440
--- /dev/null
+++ b/mlir/test/Dialect/OpenMP/memory-effects.mlir
@@ -0,0 +1,78 @@
+// RUN: mlir-opt %s --test-side-effects --verify-diagnostics
+
+func.func @sections_effects() {
+ // An implicit barrier reads and writes unspecified memory.
+ // expected-remark at +2 {{found an instance of 'read' on resource '<Default>'}}
+ // expected-remark at +1 {{found an instance of 'write' on resource '<Default>'}}
+ omp.sections {
+ omp.section {
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
+
+func.func @sections_nowait_no_effects() {
+ // expected-remark at +1 {{operation has no memory effects}}
+ omp.sections nowait {
+ omp.section {
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
+
+omp.private {type = private} @sections_p : memref<i32>
+
+func.func @sections_private_effects(%x: memref<i32>) {
+ // expected-remark at +5 {{found an instance of 'allocate' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +4 {{found an instance of 'free' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +3 {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
+ // expected-remark at +2 {{found an instance of 'read' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +1 {{found an instance of 'write' on op operand 0, on resource '<Default>'}}
+ omp.sections nowait private(@sections_p %x -> %private_x : memref<i32>) {
+ omp.section {
+ ^bb0(%section_x: memref<i32>):
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
+
+omp.declare_reduction @add_f32 : f32
+init {
+^bb0(%unused: f32):
+ // expected-remark at +1 {{operation has no memory effects}}
+ %zero = arith.constant 0.0 : f32
+ omp.yield(%zero : f32)
+}
+combiner {
+^bb0(%lhs: f32, %rhs: f32):
+ // expected-remark at +1 {{operation has no memory effects}}
+ %sum = arith.addf %lhs, %rhs : f32
+ omp.yield(%sum : f32)
+}
+
+func.func @sections_reduction_effects(%x: memref<f32>) {
+ // TODO: Nowait does not eliminate an implicit barrier if the reduction clause is also present,
+ // requiring the below two remarks
+ // expected-remark at +7 {{found an instance of 'read' on resource '<Default>'}}
+ // expected-remark at +6 {{found an instance of 'write' on resource '<Default>'}}
+ // expected-remark at +5 {{found an instance of 'allocate' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +4 {{found an instance of 'free' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +3 {{found an instance of 'read' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +2 {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
+ // expected-remark at +1 {{found an instance of 'write' on op operand 0, on resource '<Default>'}}
+ omp.sections nowait reduction(
+ @add_f32 %x -> %private_x : memref<f32>) {
+ omp.section {
+ ^bb0(%section_x: memref<f32>):
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
>From 3666bef6b66ec3c8e54bec62a4cd7f6b58a0c56d Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com"
<jamrow01 at ip-10-252-16-38.eu-west-1.compute.internal>
Date: Fri, 9 Oct 2026 09:12:52 +0000
Subject: [PATCH 3/3] Revert "Merge branch 'pr/openmp-memory-effects-sections'
of https://github.com/JRP-arm/llvm-project into
pr/openmp-memory-effects-sections"
This reverts commit 2340f288446ebff445652ccc6ddcc7e0210cb24b, reversing
changes made to 279f7e27cd31fa3ed939488ad46e14ca202bd98b.
---
.../mlir/Dialect/OpenMP/OpenMPDialect.h | 9 ---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 76 ++++++-------------
2 files changed, 23 insertions(+), 62 deletions(-)
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h b/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
index 0af6515309ad3c..a6854752f75b66 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
@@ -61,15 +61,6 @@ std::tuple<NewCliOp, OpOperand *, OpOperand *> decodeCli(mlir::Value cli);
/// OpenMP dialect enum. The LLVM 'default' and 'unknown' kinds have no dialect
/// counterpart and are not valid inputs.
ClauseProcBindKind convertProcBindKind(llvm::omp::ProcBindKind kind);
-
-/// Models shared OpenMP runtime bookkeeping used to manage threadprivate
-/// variables including caches.
-struct ThreadprivateRuntimeResource
- : SideEffects::Resource::Base<ThreadprivateRuntimeResource> {
- StringRef getName() const final { return "OpenMPThreadprivateRuntime"; }
-
- bool isAddressable() const override { return false; }
-};
} // namespace mlir::omp
#endif // MLIR_DIALECT_OPENMP_OPENMPDIALECT_H_
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 89c830dc07b329..0b70d8d75da193 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -3291,9 +3291,8 @@ static void getPrivatisationEffects(
auto privatiser =
SymbolTable::lookupNearestSymbolFrom<PrivateClauseOp>(op, privateSym);
// RecursiveMemoryEffects only covers regions nested in the directive, not
- // symbol-referenced regions, user defined types have arbitrary recipe
- // bodies. Default to a conservative model of read and write to arbitrary
- // memory.
+ // symbol-referenced regions, user defined types have arbitrary recipe bodies.
+ // Default to a conservative model of read and write to arbitrary memory.
if (!privatiser || !areRegionsMemoryEffectFree(privatiser)) {
effects.emplace_back(MemoryEffects::Read::get());
effects.emplace_back(MemoryEffects::Write::get());
@@ -3301,27 +3300,29 @@ static void getPrivatisationEffects(
}
}
-template <typename OpType>
-static void
-getLastprivateEffects(OpType op,
- SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
- auto iface = cast<BlockArgOpenMPOpInterface>(op.getOperation());
- // We cannot determine if the variable is lastprivate, so assume all private
- // variables are.
- for (auto entry : llvm::zip_equal(op.getPrivateVarsMutable(),
- iface.getPrivateBlockArgs())) {
+void SectionsOp::getEffects(
+ llvm::SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>>
+ &effects) {
+ // TODO: Nowait does not eliminate an implicit barrier if the reduction clause
+ // is also present
+ if (!getNowait() || (!getReductionVarsMutable().empty() && getNowait())) {
+ effects.emplace_back(MemoryEffects::Read::get());
+ effects.emplace_back(MemoryEffects::Write::get());
+ }
+
+ getPrivatisationEffects(*this, effects);
+ auto iface = cast<BlockArgOpenMPOpInterface>(getOperation());
+ // We cannot determine if the variable is lastprivate, so assume all privates
+ // are.
+ for (auto entry :
+ llvm::zip_equal(getPrivateVarsMutable(), iface.getPrivateBlockArgs())) {
effects.emplace_back(MemoryEffects::Read::get(), std::get<1>(entry));
effects.emplace_back(MemoryEffects::Write::get(), &std::get<0>(entry));
}
-}
-/// Model accumulator initialisation and destruction and referenced recipes for
-/// a reduction clause.
-static void
-getReductionEffects(Operation *op, MutableOperandRange reductionVars,
- ArrayRef<BlockArgument> reductionArgs,
- std::optional<ArrayAttr> reductionSyms,
- SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
+ Block::BlockArgListType reductionArgs = iface.getReductionBlockArgs();
+ MutableOperandRange reductionVars = getReductionVarsMutable();
+
for (auto entry : llvm::zip_equal(reductionVars, reductionArgs)) {
OpOperand &original = std::get<0>(entry);
BlockArgument privateAccumulator = std::get<1>(entry);
@@ -3330,16 +3331,15 @@ getReductionEffects(Operation *op, MutableOperandRange reductionVars,
effects.emplace_back(MemoryEffects::Free::get(), privateAccumulator);
effects.emplace_back(MemoryEffects::Read::get(), privateAccumulator);
- effects.emplace_back(MemoryEffects::Write::get(), privateAccumulator);
effects.emplace_back(MemoryEffects::Read::get(), &original);
effects.emplace_back(MemoryEffects::Write::get(), &original);
}
- if (reductionSyms) {
+ if (auto reductionSyms = getReductionSyms()) {
for (Attribute attr : *reductionSyms) {
auto recipe = SymbolTable::lookupNearestSymbolFrom<DeclareReductionOp>(
- op, cast<SymbolRefAttr>(attr));
+ *this, cast<SymbolRefAttr>(attr));
if (recipe && areRegionsMemoryEffectFree(recipe))
continue;
// RecursiveMemoryEffects only covers operations nested in this directive.
@@ -3352,36 +3352,6 @@ getReductionEffects(Operation *op, MutableOperandRange reductionVars,
}
}
-static void getBarrierEffects(
- llvm::SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>>
- &effects) {
- effects.emplace_back(MemoryEffects::Read::get(),
- ThreadprivateRuntimeResource::get());
- effects.emplace_back(MemoryEffects::Write::get(),
- ThreadprivateRuntimeResource::get());
-}
-
-void SectionsOp::getEffects(
- llvm::SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>>
- &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())) {
- getBarrierEffects(effects);
- }
-
- getPrivatisationEffects(*this, effects);
- getLastprivateEffects(*this, effects);
-
- auto iface = cast<BlockArgOpenMPOpInterface>(getOperation());
- getReductionEffects(*this, getReductionVarsMutable(),
- iface.getReductionBlockArgs(), getReductionSyms(),
- effects);
-}
-
//===----------------------------------------------------------------------===//
// ScopeOp
//===----------------------------------------------------------------------===//
More information about the Mlir-commits
mailing list