[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 30 08:01:27 PDT 2026
https://github.com/JRP-arm updated https://github.com/llvm/llvm-project/pull/230167
>From 5529228777a1151f806d8a459cff470dec5c649c 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 01/17] [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 | 79 ++++++++++-
6 files changed, 297 insertions(+), 6 deletions(-)
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 6aed702a2da345..bb7c57df1a5278 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
@@ -62,6 +62,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 307b91ebe99af2..d00a1fbc55d2d8 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -306,7 +306,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 = [{
@@ -329,7 +330,8 @@ def SectionOp : OpenMP_Op<"section", traits = [
def SectionsOp : OpenMP_Op<"sections", traits = [
AttrSizedOperandSegments, DeclareOpInterfaceMethods<ComposableOpInterface>,
- HasParallelRegion
+ HasParallelRegion,
+ 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 0b0d4f6f658c67..ae2d73efda7c35 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -3465,6 +3465,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
index 1efa0acd23da0e..68a503b50c186a 100644
--- a/mlir/test/Dialect/OpenMP/memory-effects.mlir
+++ b/mlir/test/Dialect/OpenMP/memory-effects.mlir
@@ -16,4 +16,81 @@ func.func @master_recursive_memory_effects() {
omp.terminator
}
return
-}
\ No newline at end of file
+}
+
+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 4688bd3da7267ce965ab98e7a3be30646c39e019 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 02/17] 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 bb7c57df1a5278..6aed702a2da345 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
@@ -62,15 +62,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 ae2d73efda7c35..18be6dde44a3a9 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -3498,9 +3498,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());
@@ -3508,27 +3507,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);
@@ -3537,16 +3538,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.
@@ -3559,36 +3559,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
//===----------------------------------------------------------------------===//
>From 6ea994f7637ec5eca00e0bb86cdcd581b5fc7224 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 03/17] 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/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 75 ++++++++++++++------
mlir/test/Dialect/OpenMP/cse.mlir | 20 ++++++
mlir/test/Dialect/OpenMP/memory-effects.mlir | 11 +--
3 files changed, 78 insertions(+), 28 deletions(-)
create mode 100644 mlir/test/Dialect/OpenMP/cse.mlir
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 18be6dde44a3a9..c28609d0c0434a 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -3492,14 +3492,16 @@ static void getPrivatisationEffects(
effects.emplace_back(MemoryEffects::Allocate::get(), privateCopy);
effects.emplace_back(MemoryEffects::Free::get(), privateCopy);
// private variables may read the original as a mold
+ // firstprivate variables do it regardless
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.
+ // 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());
@@ -3507,29 +3509,27 @@ static void getPrivatisationEffects(
}
}
-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())) {
+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));
}
+}
- Block::BlockArgListType reductionArgs = iface.getReductionBlockArgs();
- MutableOperandRange reductionVars = getReductionVarsMutable();
-
+/// 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);
@@ -3538,15 +3538,16 @@ void SectionsOp::getEffects(
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 (auto reductionSyms = getReductionSyms()) {
+ if (reductionSyms) {
for (Attribute attr : *reductionSyms) {
auto recipe = SymbolTable::lookupNearestSymbolFrom<DeclareReductionOp>(
- *this, cast<SymbolRefAttr>(attr));
+ op, cast<SymbolRefAttr>(attr));
if (recipe && areRegionsMemoryEffectFree(recipe))
continue;
// RecursiveMemoryEffects only covers operations nested in this directive.
@@ -3559,6 +3560,34 @@ void SectionsOp::getEffects(
}
}
+static void getBarrierEffects(
+ llvm::SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>>
+ &effects) {
+ effects.emplace_back(MemoryEffects::Read::get());
+ effects.emplace_back(MemoryEffects::Write::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/cse.mlir b/mlir/test/Dialect/OpenMP/cse.mlir
new file mode 100644
index 00000000000000..919547efc2a092
--- /dev/null
+++ b/mlir/test/Dialect/OpenMP/cse.mlir
@@ -0,0 +1,20 @@
+// RUN: mlir-opt %s -cse -split-input-file | FileCheck %s
+
+// An implicit barrier synchronises shared memory, so loads on opposite sides
+// must remain.
+// CHECK-LABEL: func.func @sections_barrier_between_loads(
+// CHECK: %[[BEFORE:.*]] = memref.load %[[SHARED:.*]][] : memref<i32>
+// CHECK: omp.sections {
+// CHECK: %[[AFTER:.*]] = memref.load %[[SHARED]][] : memref<i32>
+// CHECK: return %[[BEFORE]], %[[AFTER]] : i32, i32
+func.func @sections_barrier_between_loads(%shared: memref<i32>) -> (i32, i32) {
+ %before = memref.load %shared[] : memref<i32>
+ omp.sections {
+ omp.section {
+ omp.terminator
+ }
+ omp.terminator
+ }
+ %after = memref.load %shared[] : memref<i32>
+ return %before, %after : i32, i32
+}
diff --git a/mlir/test/Dialect/OpenMP/memory-effects.mlir b/mlir/test/Dialect/OpenMP/memory-effects.mlir
index 68a503b50c186a..79346ae2e8aae9 100644
--- a/mlir/test/Dialect/OpenMP/memory-effects.mlir
+++ b/mlir/test/Dialect/OpenMP/memory-effects.mlir
@@ -77,11 +77,12 @@ combiner {
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 +8 {{found an instance of 'read' on resource '<Default>'}}
+ // expected-remark at +7 {{found an instance of 'write' on resource '<Default>'}}
+ // expected-remark at +6 {{found an instance of 'allocate' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +5 {{found an instance of 'free' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +4 {{found an instance of 'read' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +3 {{found an instance of 'write' 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(
>From 2ef6e7663ebf1dbd3ff761a0ad681787a0566cd7 Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com"
<jamrow01 at ip-10-249-51-4.eu-west-1.compute.internal>
Date: Mon, 12 Oct 2026 07:55:52 +0000
Subject: [PATCH 04/17] Improve readability of loop variables
---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 31 ++++++++------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index c28609d0c0434a..e2f5afe7407cfe 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -3485,19 +3485,15 @@ static void getPrivatisationEffects(
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
+ for (auto [sourceVar, privateVar, privateSym] : llvm::zip_equal(privateVars, privateArgs, *privateSyms)) {
+ effects.emplace_back(MemoryEffects::Allocate::get(), privateVar);
+ effects.emplace_back(MemoryEffects::Free::get(), privateVar);
+ // private variables may read the sourceVar as a mold
// firstprivate variables do it regardless
- effects.emplace_back(MemoryEffects::Read::get(), &original);
+ effects.emplace_back(MemoryEffects::Read::get(), &sourceVar);
- auto privateSym = cast<SymbolRefAttr>(std::get<2>(entry));
auto privatiser =
- SymbolTable::lookupNearestSymbolFrom<PrivateClauseOp>(op, privateSym);
+ SymbolTable::lookupNearestSymbolFrom<PrivateClauseOp>(op, cast<SymbolRefAttr>(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
@@ -3516,10 +3512,10 @@ getLastprivateEffects(OpType op,
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(),
+ for (auto [sourceVar, privateVar] : 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));
+ effects.emplace_back(MemoryEffects::Read::get(), privateVar);
+ effects.emplace_back(MemoryEffects::Write::get(), &sourceVar);
}
}
@@ -3530,18 +3526,15 @@ 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);
-
+ for (auto [sourceVar, privateAccumulator] : llvm::zip_equal(reductionVars, reductionArgs)) {
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);
+ effects.emplace_back(MemoryEffects::Read::get(), &sourceVar);
+ effects.emplace_back(MemoryEffects::Write::get(), &sourceVar);
}
if (reductionSyms) {
>From b63c1fb64e5c172f59a84f922c7d353373d751f7 Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com"
<jamrow01 at ip-10-249-51-4.eu-west-1.compute.internal>
Date: Mon, 12 Oct 2026 14:06:58 +0000
Subject: [PATCH 05/17] Improve accuracy of modelling memory effects of
reduction and private recipe bodies.
Add tests for tracing recipe arguments through subviews and nested effects
---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 222 +++++++++++++------
mlir/test/Dialect/OpenMP/memory-effects.mlir | 165 +++++++++++---
2 files changed, 294 insertions(+), 93 deletions(-)
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index e2f5afe7407cfe..b29c1ff5d79b38 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -24,6 +24,7 @@
#include "mlir/IR/SymbolTable.h"
#include "mlir/Interfaces/FoldInterfaces.h"
#include "mlir/Interfaces/SideEffectInterfaces.h"
+#include "mlir/Interfaces/ViewLikeInterface.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/PostOrderIterator.h"
@@ -3465,91 +3466,181 @@ 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;
+using RecipeEffectTarget = std::variant<OpOperand *, BlockArgument>;
+
+/// Follow views to the underlying value, which may be a recipe argument.
+static Value getRecipeEffectBase(Value value) {
+ while (value) {
+ auto view = value.getDefiningOp<ViewLikeOpInterface>();
+ if (!view || value != view.getViewDest())
+ break;
+ value = view.getViewSource();
+ }
+ return value;
}
-/// Model private storage and the effects of referenced privatisation recipes.
+/// Append argument mappings for private variable recipes.
+static void populateRecipeEffectMapping(
+ PrivateClauseOp recipe, OpOperand &sourceVar, BlockArgument privateVar,
+ SmallVectorImpl<std::pair<Value, RecipeEffectTarget>> &mapping) {
+ auto add = [&](BlockArgument arg, RecipeEffectTarget target) {
+ if (arg)
+ mapping.emplace_back(arg, target);
+ };
+
+ add(recipe.getInitMoldArg(), &sourceVar);
+ add(recipe.getInitPrivateArg(), privateVar);
+ add(recipe.getCopyMoldArg(), &sourceVar);
+ add(recipe.getCopyPrivateArg(), privateVar);
+ add(recipe.getDeallocMoldArg(), privateVar);
+}
+
+/// Append argument mappings for a reduction recipe.
+static void populateRecipeEffectMapping(
+ DeclareReductionOp recipe, OpOperand &sourceVar,
+ BlockArgument privateAccumulator,
+ SmallVectorImpl<std::pair<Value, RecipeEffectTarget>> &mapping) {
+ auto mapRegion = [&](Region ®ion, ArrayRef<RecipeEffectTarget> targets) {
+ if (region.empty())
+ return;
+
+ for (auto [arg, target] : llvm::zip(region.getArguments(), targets))
+ mapping.emplace_back(arg, target);
+ };
+
+ mapRegion(recipe.getAllocRegion(), {&sourceVar});
+ // Not all combiner recipes are the same, some arguments may not always
+ // be the same variable.
+ mapRegion(recipe.getInitializerRegion(), {&sourceVar, privateAccumulator});
+ mapRegion(recipe.getReductionRegion(), {&sourceVar, privateAccumulator});
+ mapping.emplace_back(recipe.getReductionLhsArg(), privateAccumulator);
+ mapRegion(recipe.getAtomicReductionRegion(),
+ {&sourceVar, privateAccumulator});
+ mapRegion(recipe.getCleanupRegion(), {privateAccumulator});
+}
+
+/// Add arbitrary read, write, allocate, and free effects.
+static void addUnknownMemoryEffects(
+ SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
+ effects.emplace_back(MemoryEffects::Read::get());
+ effects.emplace_back(MemoryEffects::Write::get());
+ effects.emplace_back(MemoryEffects::Allocate::get());
+ effects.emplace_back(MemoryEffects::Free::get());
+}
+
+/// Add the effects of a symbol-referenced recipe to the corresponding
+/// variables in its directive.
+static void
+getRecipeEffects(Operation *recipe,
+ ArrayRef<std::pair<Value, RecipeEffectTarget>> mapping,
+ SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
+ for (Region ®ion : recipe->getRegions()) {
+ region.walk([&](Operation *op) {
+ auto recipeEffects = dyn_cast<MemoryEffectOpInterface>(op);
+ if (!recipeEffects) {
+ if (hasUnknownEffects(op)) {
+ // An operation without known effects may access arbitrary memory.
+ addUnknownMemoryEffects(effects);
+ }
+ return;
+ }
+
+ SmallVector<MemoryEffects::EffectInstance> opEffects;
+ recipeEffects.getEffects(opEffects);
+ for (const MemoryEffects::EffectInstance &effect : opEffects) {
+ Value affectedValue = effect.getValue();
+ Value base = getRecipeEffectBase(effect.getValue());
+ bool affectsFullRegion =
+ effect.getEffectOnFullRegion() && affectedValue == base;
+ bool mapped = false;
+ for (const auto &[arg, target] : mapping) {
+ // Check if the operand corresponds to a recipe argument
+ if (arg != base)
+ continue;
+ mapped = true;
+ std::visit(
+ [&](auto mappedValue) {
+ effects.emplace_back(effect.getEffect(), mappedValue,
+ effect.getParameters(), affectsFullRegion,
+ effect.getResource());
+ },
+ target);
+ }
+ if (!mapped) {
+ effects.emplace_back(effect.getEffect(), effect.getParameters(),
+ false, effect.getResource());
+ }
+ }
+ });
+ }
+}
+
+/// Model private storage and the effects of privatisation recipes.
template <typename OpType>
static void getPrivatisationEffects(
OpType op, SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
+ auto iface = cast<BlockArgOpenMPOpInterface>(op.getOperation());
+ MutableOperandRange privateVars = op.getPrivateVarsMutable();
+ Block::BlockArgListType privateArgs = iface.getPrivateBlockArgs();
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 [sourceVar, privateVar, privateSym] : llvm::zip_equal(privateVars, privateArgs, *privateSyms)) {
+ for (auto [sourceVar, privateVar, privateSym] :
+ llvm::zip_equal(privateVars, privateArgs, *privateSyms)) {
+ // Storage is allocated by the directive even if the recipe has no regions.
+ // Implicit stack storage is released by the enclosing allocation scope.
effects.emplace_back(MemoryEffects::Allocate::get(), privateVar);
- effects.emplace_back(MemoryEffects::Free::get(), privateVar);
- // private variables may read the sourceVar as a mold
- // firstprivate variables do it regardless
- effects.emplace_back(MemoryEffects::Read::get(), &sourceVar);
-
- auto privatiser =
- SymbolTable::lookupNearestSymbolFrom<PrivateClauseOp>(op, cast<SymbolRefAttr>(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());
+
+ auto privatiser = SymbolTable::lookupNearestSymbolFrom<PrivateClauseOp>(
+ op, cast<SymbolRefAttr>(privateSym));
+ if (!privatiser) {
+ continue;
}
- }
-}
-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 [sourceVar, privateVar] : llvm::zip_equal(op.getPrivateVarsMutable(),
- iface.getPrivateBlockArgs())) {
- effects.emplace_back(MemoryEffects::Read::get(), privateVar);
- effects.emplace_back(MemoryEffects::Write::get(), &sourceVar);
+ SmallVector<std::pair<Value, RecipeEffectTarget>> mapping;
+ populateRecipeEffectMapping(privatiser, sourceVar, privateVar, mapping);
+ getRecipeEffects(privatiser, mapping, effects);
}
}
/// Model accumulator initialisation and destruction and referenced recipes for
/// a reduction clause.
+template <typename OpType>
static void
-getReductionEffects(Operation *op, MutableOperandRange reductionVars,
- ArrayRef<BlockArgument> reductionArgs,
- std::optional<ArrayAttr> reductionSyms,
+getReductionEffects(OpType op,
SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
- for (auto [sourceVar, privateAccumulator] : llvm::zip_equal(reductionVars, reductionArgs)) {
- effects.emplace_back(MemoryEffects::Allocate::get(), privateAccumulator);
- effects.emplace_back(MemoryEffects::Free::get(), privateAccumulator);
+ auto iface = cast<BlockArgOpenMPOpInterface>(op.getOperation());
+ auto reductionVars = op.getReductionVarsMutable();
+ auto reductionArgs = iface.getReductionBlockArgs();
+ auto reductionSyms = op.getReductionSyms();
+ auto reductionByref = op.getReductionByref();
- effects.emplace_back(MemoryEffects::Read::get(), privateAccumulator);
- effects.emplace_back(MemoryEffects::Write::get(), privateAccumulator);
+ if (!reductionSyms)
+ return;
- effects.emplace_back(MemoryEffects::Read::get(), &sourceVar);
- effects.emplace_back(MemoryEffects::Write::get(), &sourceVar);
- }
+ for (auto [index, sourceVar, privateAccumulator, recipeSym] :
+ llvm::enumerate(reductionVars, reductionArgs, *reductionSyms)) {
+ if (!reductionByref || !(*reductionByref)[index]) {
+ // By-value recipes operate on scalar values. The directive allocates
+ // accumulator storage and loads/stores values around recipe invocations.
+ // By-reference recipes account for these effects in their bodies.
+ effects.emplace_back(MemoryEffects::Allocate::get(), privateAccumulator);
+ effects.emplace_back(MemoryEffects::Read::get(), &sourceVar);
+ effects.emplace_back(MemoryEffects::Write::get(), privateAccumulator);
+ effects.emplace_back(MemoryEffects::Read::get(), privateAccumulator);
+ effects.emplace_back(MemoryEffects::Write::get(), &sourceVar);
+ }
- 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;
+ auto recipe = SymbolTable::lookupNearestSymbolFrom<DeclareReductionOp>(
+ op, cast<SymbolRefAttr>(recipeSym));
+ if (!recipe) {
+ continue;
}
+
+ SmallVector<std::pair<Value, RecipeEffectTarget>> mapping;
+ populateRecipeEffectMapping(recipe, sourceVar, privateAccumulator, mapping);
+ getRecipeEffects(recipe, mapping, effects);
}
}
@@ -3573,12 +3664,7 @@ void SectionsOp::getEffects(
}
getPrivatisationEffects(*this, effects);
- getLastprivateEffects(*this, effects);
-
- auto iface = cast<BlockArgOpenMPOpInterface>(getOperation());
- getReductionEffects(*this, getReductionVarsMutable(),
- iface.getReductionBlockArgs(), getReductionSyms(),
- effects);
+ getReductionEffects(*this, effects);
}
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/OpenMP/memory-effects.mlir b/mlir/test/Dialect/OpenMP/memory-effects.mlir
index 79346ae2e8aae9..72cd9dc916f692 100644
--- a/mlir/test/Dialect/OpenMP/memory-effects.mlir
+++ b/mlir/test/Dialect/OpenMP/memory-effects.mlir
@@ -31,25 +31,11 @@ func.func @sections_effects() {
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>'}}
+func.func @sections_private_implicit_allocation(%x: memref<i32>) {
+ // The directive allocates private storage even though the recipe is empty.
+ // expected-remark at +1 {{found an instance of 'allocate' on block argument 0, on resource '<Default>'}}
omp.sections nowait private(@sections_p %x -> %private_x : memref<i32>) {
omp.section {
^bb0(%section_x: memref<i32>):
@@ -75,15 +61,15 @@ combiner {
}
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 +8 {{found an instance of 'read' on resource '<Default>'}}
- // expected-remark at +7 {{found an instance of 'write' on resource '<Default>'}}
- // expected-remark at +6 {{found an instance of 'allocate' on block argument 0, on resource '<Default>'}}
- // expected-remark at +5 {{found an instance of 'free' on block argument 0, on resource '<Default>'}}
- // expected-remark at +4 {{found an instance of 'read' on block argument 0, on resource '<Default>'}}
+ // The first two effects come from the implicit barrier, which nowait does
+ // not currently eliminate when a reduction clause is present. Allocation,
+ // initialization and combination access memory outside the by-value recipe.
+ // 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 'read' on op operand 0, on resource '<Default>'}}
// expected-remark at +3 {{found an instance of 'write' 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 +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 reduction(
@add_f32 %x -> %private_x : memref<f32>) {
@@ -95,3 +81,132 @@ func.func @sections_reduction_effects(%x: memref<f32>) {
}
return
}
+
+// Effects nested in the copy region must be traced through subviews to the
+// original mold and private arguments.
+omp.private {type = firstprivate} @sections_copy_subviews : memref<4xi32> copy {
+^bb0(%mold: memref<4xi32>, %private: memref<4xi32>):
+ // Use scf.execute_region to test recursive effects.
+ scf.execute_region {
+ // expected-remark at +1 {{operation has no memory effects}}
+ %c0 = arith.constant 0 : index
+ // expected-remark at +1 {{operation has no memory effects}}
+ %mold_view = memref.subview %mold[1] [2] [1] : memref<4xi32> to memref<2xi32, strided<[1], offset: 1>>
+ // expected-remark at +1 {{operation has no memory effects}}
+ %private_view = memref.subview %private[1] [2] [1] : memref<4xi32> to memref<2xi32, strided<[1], offset: 1>>
+ // expected-remark at +1 {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
+ %value = memref.load %mold_view[%c0] : memref<2xi32, strided<[1], offset: 1>>
+ // expected-remark at +1 {{found an instance of 'write' on op operand 1, on resource '<Default>'}}
+ memref.store %value, %private_view[%c0] : memref<2xi32, strided<[1], offset: 1>>
+ scf.yield
+ }
+ omp.yield(%private : memref<4xi32>)
+} dealloc {
+^bb0(%private: memref<4xi32>):
+ // expected-remark at +1 {{found an instance of 'free' on op operand 0, on resource '<Default>'}}
+ memref.dealloc %private : memref<4xi32>
+ omp.yield
+}
+
+func.func @sections_private_subview_effects(%x: memref<4xi32>) {
+ // expected-remark at +4 {{found an instance of 'allocate' 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 'write' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +1 {{found an instance of 'free' on block argument 0, on resource '<Default>'}}
+ omp.sections nowait private(@sections_copy_subviews %x -> %private_x : memref<4xi32>) {
+ omp.section {
+ ^bb0(%section_x: memref<4xi32>):
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
+
+// Model allocation, initialisation, combination and cleanup effects.
+// The allocation is local to the recipe initialiser.
+// The combiner LHS can be the source or a private accumulator.
+omp.declare_reduction @sections_add_byref : memref<f32>
+alloc {
+ // expected-remark at +1 {{found an instance of 'allocate' on op result 0, on resource '<Default>'}}
+ %allocated = memref.alloc() : memref<f32>
+ omp.yield(%allocated : memref<f32>)
+}
+init {
+^bb0(%mold: memref<f32>, %private: memref<f32>):
+ // expected-remark at +1 {{operation has no memory effects}}
+ %zero = arith.constant 0.0 : f32
+ // expected-remark at +1 {{found an instance of 'write' on op operand 1, on resource '<Default>'}}
+ memref.store %zero, %private[] : memref<f32>
+ omp.yield(%private : memref<f32>)
+}
+combiner {
+^bb0(%lhs: memref<f32>, %rhs: memref<f32>):
+ // expected-remark at +1 {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
+ %left = memref.load %lhs[] : memref<f32>
+ // expected-remark at +1 {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
+ %right = memref.load %rhs[] : memref<f32>
+ // expected-remark at +1 {{operation has no memory effects}}
+ %sum = arith.addf %left, %right : f32
+ // expected-remark at +1 {{found an instance of 'write' on op operand 1, on resource '<Default>'}}
+ memref.store %sum, %lhs[] : memref<f32>
+ omp.yield(%lhs : memref<f32>)
+}
+cleanup {
+^bb0(%private: memref<f32>):
+ // expected-remark at +1 {{found an instance of 'free' on op operand 0, on resource '<Default>'}}
+ memref.dealloc %private : memref<f32>
+ omp.yield
+}
+
+func.func @sections_reduction_recipe_effects(%x: memref<f32>) {
+ // The first two effects come from the implicit barrier.
+ // expected-remark at +8 {{found an instance of 'read' on resource '<Default>'}}
+ // expected-remark at +7 {{found an instance of 'write' on resource '<Default>'}}
+ // expected-remark at +6 {{found an instance of 'allocate' on resource '<Default>'}}
+ // expected-remark at +5 {{found an instance of 'write' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +4 {{found an instance of 'read' on op operand 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 'write' on op operand 0, on resource '<Default>'}}
+ // expected-remark at +1 {{found an instance of 'free' on block argument 0, on resource '<Default>'}}
+ omp.sections nowait reduction(byref @sections_add_byref %x -> %private_x : memref<f32>) {
+ omp.section {
+ ^bb0(%section_x: memref<f32>):
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
+
+// Unknown effects inside a nested region require an untargeted read and write,
+// while retaining known effects already collected from the recipe.
+func.func private @unknown_recipe_effects(memref<i32>, i32)
+
+omp.private {type = firstprivate} @sections_unknown_copy : memref<i32> copy {
+^bb0(%mold: memref<i32>, %private: memref<i32>):
+ // expected-remark at +1 {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
+ %value = memref.load %mold[] : memref<i32>
+ scf.execute_region {
+ func.call @unknown_recipe_effects(%private, %value) : (memref<i32>, i32) -> ()
+ scf.yield
+ }
+ omp.yield(%private : memref<i32>)
+}
+
+func.func @sections_private_unknown_recipe_effects(%x: memref<i32>) {
+ // expected-remark at +6 {{found an instance of 'allocate' on block argument 0, on resource '<Default>'}}
+ // expected-remark at +5 {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
+ // expected-remark at +4 {{found an instance of 'read' on resource '<Default>'}}
+ // expected-remark at +3 {{found an instance of 'write' on resource '<Default>'}}
+ // expected-remark at +2 {{found an instance of 'allocate' on resource '<Default>'}}
+ // expected-remark at +1 {{found an instance of 'free' on resource '<Default>'}}
+ omp.sections nowait private(@sections_unknown_copy %x -> %private_x : memref<i32>) {
+ omp.section {
+ ^bb0(%section_x: memref<i32>):
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
>From ee7f58a37b4d3df4fffff7faaf67a6bf847821df Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com" <jamie.rowlinson-patel at arm.com>
Date: Fri, 16 Oct 2026 13:02:52 +0000
Subject: [PATCH 06/17] Revert to previous conservative memory effects. For
non-derived types insert a flag which assumes a bound on recipe memory
effects
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 37 +++
.../Lower/OpenMP/recipe-memory-effects.f90 | 100 +++++++
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 31 ++-
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 183 ++-----------
mlir/test/Dialect/OpenMP/memory-effects.mlir | 251 ++++++++++--------
5 files changed, 325 insertions(+), 277 deletions(-)
create mode 100644 flang/test/Lower/OpenMP/recipe-memory-effects.f90
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 3ce793989dee30..dd6956e6cd728c 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3564,6 +3564,28 @@ findEnclosingParallelOp(fir::FirOpBuilder &builder) {
return {};
}
+static bool isNonDerived(const semantics::Symbol &sym) {
+ const auto *type = sym.GetUltimate().GetType();
+ return type && type->AsIntrinsic();
+}
+
+static bool isNonDerivedItem(const semantics::Symbol *sym) {
+ const auto &ultimate = sym->GetUltimate();
+ if (const auto *common =
+ ultimate.detailsIf<semantics::CommonBlockDetails>()) {
+ return llvm::all_of(common->objects(), [](const auto &member) {
+ return isNonDerived(*member);
+ });
+ }
+ return isNonDerived(ultimate);
+}
+
+static bool isDerivedVariable(mlir::Value variable) {
+ mlir::Type type = variable.getType();
+ return !fir::isa_derived(fir::getFortranElementType(type)) &&
+ !fir::isa_volatile_type(type);
+}
+
static mlir::omp::SectionsOp
genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
@@ -3657,6 +3679,21 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
// SECTIONS construct.
auto sectionsOp = mlir::omp::SectionsOp::create(builder, loc, clauseOps);
+ // Becuase Flang uses eager privatisation clausOps.privateVars is empty.
+ bool allPrivateVarsEligible =
+ llvm::all_of(dsp.getAllSymbolsToPrivatize(), isNonDerivedItem);
+
+ bool allReductionVarsEligible =
+ llvm::all_of(clauseOps.reductionVars, [lpAlloca](mlir::Value variable) {
+ // The generated conditional-lastprivate recipe only accesses the
+ // intrinsic value and index fields of its accumulator, despite its
+ // record type.
+ return variable == lpAlloca ||
+ isDerivedVariable(variable);
+ });
+
+ if (allPrivateVarsEligible && allReductionVarsEligible)
+ sectionsOp.setAssumeBoundedRecipeEffects(true);
// Create entry block with reduction variables as arguments.
ObjectEntryBlockArgs args;
diff --git a/flang/test/Lower/OpenMP/recipe-memory-effects.f90 b/flang/test/Lower/OpenMP/recipe-memory-effects.f90
new file mode 100644
index 00000000000000..511a31adf2221a
--- /dev/null
+++ b/flang/test/Lower/OpenMP/recipe-memory-effects.f90
@@ -0,0 +1,100 @@
+! RUN: bbc -emit-hlfir -fopenmp -fopenmp-version=50 %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %s -o - | FileCheck %s
+
+! The bounded-recipe-effects assumption is selected using the types of all
+! reduction and privatized variables.
+module recipe_memory_effects
+ implicit none
+ type :: record
+ sequence
+ integer :: value
+ end type
+contains
+ ! Scalar and boxed intrinsic reductions and privatization are eligible.
+ ! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPintrinsic_clauses(
+ ! CHECK: omp.sections reduction(
+ ! CHECK: } <assume_bounded_recipe_effects>
+ subroutine intrinsic_clauses(x, y, p, f, l)
+ integer :: x, p, f, l
+ real :: y
+ !$omp sections reduction(+:x,y) private(p) firstprivate(f) lastprivate(l)
+ p = f
+ l = p
+ x = x + p
+ y = y + 1.0
+ !$omp end sections
+ end subroutine
+
+ ! A derived private variable prevents the assumption.
+ ! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPderived_private(
+ ! CHECK: omp.sections {
+ ! CHECK-NOT: assume_bounded_recipe_effects
+ ! CHECK: return
+ subroutine derived_private()
+ integer :: i
+ type(record) :: z
+ !$omp sections private(i, z)
+ i = 1
+ z%value = i
+ !$omp end sections
+ end subroutine
+
+ ! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPintrinsic_private(
+ ! CHECK: omp.sections {
+ ! CHECK: } <assume_bounded_recipe_effects>
+ subroutine intrinsic_private()
+ integer :: x
+ real :: y
+ !$omp sections private(x, y)
+ x = 1
+ y = 1.0
+ !$omp end sections
+ end subroutine
+
+ ! The generated conditional-lastprivate record is eligible.
+ ! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPconditional_lastprivate(
+ ! CHECK: omp.sections reduction(
+ ! CHECK: } <assume_bounded_recipe_effects>
+ subroutine conditional_lastprivate(x, y)
+ integer :: x, y
+ !$omp sections lastprivate(conditional:x) firstprivate(x) reduction(+:y)
+ x = x + 1
+ y = y + x
+ !$omp end sections
+ end subroutine
+
+ ! A user-derived reduction still prevents the assumption.
+ ! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPconditional_lastprivate_derived_reduction(
+ ! CHECK: omp.sections reduction(byref
+ ! CHECK-NOT: assume_bounded_recipe_effects
+ ! CHECK: return
+ subroutine conditional_lastprivate_derived_reduction(x, z)
+ integer :: x
+ type(record) :: z
+ !$omp declare reduction(record_sum : record : &
+ !$omp & omp_out%value = omp_out%value + omp_in%value) &
+ !$omp & initializer(omp_priv%value = 0)
+ !$omp parallel
+ !$omp sections lastprivate(conditional:x) reduction(record_sum:z)
+ x = 1
+ z%value = z%value + x
+ !$omp end sections
+ !$omp end parallel
+ end subroutine
+
+ ! A derived private variable still prevents the assumption.
+ ! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPconditional_lastprivate_derived_private(
+ ! CHECK: omp.sections reduction(byref
+ ! CHECK-NOT: assume_bounded_recipe_effects
+ ! CHECK: return
+ subroutine conditional_lastprivate_derived_private(x, z)
+ integer :: x
+ type(record) :: z
+ !$omp parallel
+ !$omp sections lastprivate(conditional:x) private(z)
+ z%value = 1
+ x = z%value
+ !$omp end sections
+ !$omp end parallel
+ end subroutine
+end module
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index d00a1fbc55d2d8..6cad7e3fceafd7 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -345,8 +345,37 @@ def SectionsOp : OpenMP_Op<"sections", traits = [
implicit task.
Block arguments for reduction variables should be mirrored in enclosed
`omp.section` operations.
+
+ The `assume_bounded_recipe_effects` flag makes the memory-effect interface
+ assume that all regions of the referenced privatisation and reduction
+ recipes obey the following bounds at this use:
+ - Privatisation may read the original variable as a mold or firstprivate
+ copy source, and may allocate, write and free private storage. It
+ may not write or free the original variable. Lastprivate copy-back is
+ represented by explicit nested operations.
+ - Reduction may read and write the original variable. It may allocate,
+ free, read or write private accumulators.
+ - These effects apply only to storage directly designated by the original
+ variables and their corresponding private block arguments.
+ - They may not access other storage, including globals or pointees reached
+ by loading pointers from these variables. Such accesses, or allocations
+ and frees of other storage, are outside the assumed bounds.
+
+ These are upper bounds, not a claim that every recipe performs every
+ permitted effect. The flag applies only to implicit recipe effects;
+ explicit nested operations and barrier effects are modelled separately.
+ Without the flag, referenced recipes are assumed to have arbitrary memory
+ effects.
+
+ Flang currently sets it when all private and reduction variables are
+ non-derived. These checks do not establish that all recipe effects satisfy
+ the bounds: user-defined reductions may exceed them.
}] # clausesDescription;
+ let arguments = !con(clausesArgs, (ins
+ UnitAttr:$assume_bounded_recipe_effects
+ ));
+
// Override region definition.
let regions = (region SizedRegion<1>:$region);
@@ -357,7 +386,7 @@ def SectionsOp : OpenMP_Op<"sections", traits = [
let assemblyFormat = clausesAssemblyFormat # [{
custom<PrivateReductionRegion>($region, $private_vars, type($private_vars),
$private_syms, $private_needs_barrier, $reduction_mod, $reduction_vars,
- type($reduction_vars), $reduction_byref, $reduction_syms) attr-dict
+ type($reduction_vars), $reduction_byref, $reduction_syms) prop-dict attr-dict
}];
let hasVerifier = 1;
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index b29c1ff5d79b38..617a4d79b49416 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -24,7 +24,6 @@
#include "mlir/IR/SymbolTable.h"
#include "mlir/Interfaces/FoldInterfaces.h"
#include "mlir/Interfaces/SideEffectInterfaces.h"
-#include "mlir/Interfaces/ViewLikeInterface.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/PostOrderIterator.h"
@@ -3438,7 +3437,8 @@ void SectionsOp::build(OpBuilder &builder, OperationState &state,
/*private_syms=*/nullptr, /*private_needs_barrier=*/nullptr,
clauses.reductionMod, clauses.reductionVars,
makeDenseBoolArrayAttr(ctx, clauses.reductionByref),
- makeArrayAttr(ctx, clauses.reductionSyms));
+ makeArrayAttr(ctx, clauses.reductionSyms),
+ /*assume_bounded_recipe_effects=*/nullptr);
}
LogicalResult SectionsOp::verify() {
@@ -3466,59 +3466,6 @@ LogicalResult SectionsOp::verifyRegions() {
return success();
}
-using RecipeEffectTarget = std::variant<OpOperand *, BlockArgument>;
-
-/// Follow views to the underlying value, which may be a recipe argument.
-static Value getRecipeEffectBase(Value value) {
- while (value) {
- auto view = value.getDefiningOp<ViewLikeOpInterface>();
- if (!view || value != view.getViewDest())
- break;
- value = view.getViewSource();
- }
- return value;
-}
-
-/// Append argument mappings for private variable recipes.
-static void populateRecipeEffectMapping(
- PrivateClauseOp recipe, OpOperand &sourceVar, BlockArgument privateVar,
- SmallVectorImpl<std::pair<Value, RecipeEffectTarget>> &mapping) {
- auto add = [&](BlockArgument arg, RecipeEffectTarget target) {
- if (arg)
- mapping.emplace_back(arg, target);
- };
-
- add(recipe.getInitMoldArg(), &sourceVar);
- add(recipe.getInitPrivateArg(), privateVar);
- add(recipe.getCopyMoldArg(), &sourceVar);
- add(recipe.getCopyPrivateArg(), privateVar);
- add(recipe.getDeallocMoldArg(), privateVar);
-}
-
-/// Append argument mappings for a reduction recipe.
-static void populateRecipeEffectMapping(
- DeclareReductionOp recipe, OpOperand &sourceVar,
- BlockArgument privateAccumulator,
- SmallVectorImpl<std::pair<Value, RecipeEffectTarget>> &mapping) {
- auto mapRegion = [&](Region ®ion, ArrayRef<RecipeEffectTarget> targets) {
- if (region.empty())
- return;
-
- for (auto [arg, target] : llvm::zip(region.getArguments(), targets))
- mapping.emplace_back(arg, target);
- };
-
- mapRegion(recipe.getAllocRegion(), {&sourceVar});
- // Not all combiner recipes are the same, some arguments may not always
- // be the same variable.
- mapRegion(recipe.getInitializerRegion(), {&sourceVar, privateAccumulator});
- mapRegion(recipe.getReductionRegion(), {&sourceVar, privateAccumulator});
- mapping.emplace_back(recipe.getReductionLhsArg(), privateAccumulator);
- mapRegion(recipe.getAtomicReductionRegion(),
- {&sourceVar, privateAccumulator});
- mapRegion(recipe.getCleanupRegion(), {privateAccumulator});
-}
-
/// Add arbitrary read, write, allocate, and free effects.
static void addUnknownMemoryEffects(
SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
@@ -3528,119 +3475,36 @@ static void addUnknownMemoryEffects(
effects.emplace_back(MemoryEffects::Free::get());
}
-/// Add the effects of a symbol-referenced recipe to the corresponding
-/// variables in its directive.
-static void
-getRecipeEffects(Operation *recipe,
- ArrayRef<std::pair<Value, RecipeEffectTarget>> mapping,
- SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
- for (Region ®ion : recipe->getRegions()) {
- region.walk([&](Operation *op) {
- auto recipeEffects = dyn_cast<MemoryEffectOpInterface>(op);
- if (!recipeEffects) {
- if (hasUnknownEffects(op)) {
- // An operation without known effects may access arbitrary memory.
- addUnknownMemoryEffects(effects);
- }
- return;
- }
-
- SmallVector<MemoryEffects::EffectInstance> opEffects;
- recipeEffects.getEffects(opEffects);
- for (const MemoryEffects::EffectInstance &effect : opEffects) {
- Value affectedValue = effect.getValue();
- Value base = getRecipeEffectBase(effect.getValue());
- bool affectsFullRegion =
- effect.getEffectOnFullRegion() && affectedValue == base;
- bool mapped = false;
- for (const auto &[arg, target] : mapping) {
- // Check if the operand corresponds to a recipe argument
- if (arg != base)
- continue;
- mapped = true;
- std::visit(
- [&](auto mappedValue) {
- effects.emplace_back(effect.getEffect(), mappedValue,
- effect.getParameters(), affectsFullRegion,
- effect.getResource());
- },
- target);
- }
- if (!mapped) {
- effects.emplace_back(effect.getEffect(), effect.getParameters(),
- false, effect.getResource());
- }
- }
- });
- }
-}
-
-/// Model private storage and the effects of privatisation recipes.
-template <typename OpType>
+/// Privatisation may read the original as a mold or firstprivate copy source.
+/// Any lastprivate copy-back is explicit in the nested operations.
static void getPrivatisationEffects(
- OpType op, SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
+ SectionsOp op, SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
auto iface = cast<BlockArgOpenMPOpInterface>(op.getOperation());
- MutableOperandRange privateVars = op.getPrivateVarsMutable();
- Block::BlockArgListType privateArgs = iface.getPrivateBlockArgs();
- std::optional<ArrayAttr> privateSyms = op.getPrivateSyms();
- if (!privateSyms)
- return;
-
- for (auto [sourceVar, privateVar, privateSym] :
- llvm::zip_equal(privateVars, privateArgs, *privateSyms)) {
- // Storage is allocated by the directive even if the recipe has no regions.
- // Implicit stack storage is released by the enclosing allocation scope.
- effects.emplace_back(MemoryEffects::Allocate::get(), privateVar);
-
- auto privatiser = SymbolTable::lookupNearestSymbolFrom<PrivateClauseOp>(
- op, cast<SymbolRefAttr>(privateSym));
- if (!privatiser) {
- continue;
- }
-
- SmallVector<std::pair<Value, RecipeEffectTarget>> mapping;
- populateRecipeEffectMapping(privatiser, sourceVar, privateVar, mapping);
- getRecipeEffects(privatiser, mapping, effects);
+ for (auto [source, priv] : llvm::zip_equal(op.getPrivateVarsMutable(),
+ iface.getPrivateBlockArgs())) {
+ effects.emplace_back(MemoryEffects::Read::get(), &source);
+ effects.emplace_back(MemoryEffects::Write::get(), priv);
+ effects.emplace_back(MemoryEffects::Allocate::get(), priv);
+ effects.emplace_back(MemoryEffects::Free::get(), priv);
}
}
-/// Model accumulator initialisation and destruction and referenced recipes for
-/// a reduction clause.
-template <typename OpType>
+/// Reductions initialise private accumulators and combine them into the
+/// original variables.
static void
-getReductionEffects(OpType op,
+getReductionEffects(SectionsOp op,
SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
auto iface = cast<BlockArgOpenMPOpInterface>(op.getOperation());
- auto reductionVars = op.getReductionVarsMutable();
- auto reductionArgs = iface.getReductionBlockArgs();
- auto reductionSyms = op.getReductionSyms();
- auto reductionByref = op.getReductionByref();
-
- if (!reductionSyms)
- return;
+ for (auto [source, priv] : llvm::zip_equal(op.getReductionVarsMutable(),
+ iface.getReductionBlockArgs())) {
+ effects.emplace_back(MemoryEffects::Read::get(), &source);
+ effects.emplace_back(MemoryEffects::Write::get(), &source);
- for (auto [index, sourceVar, privateAccumulator, recipeSym] :
- llvm::enumerate(reductionVars, reductionArgs, *reductionSyms)) {
- if (!reductionByref || !(*reductionByref)[index]) {
- // By-value recipes operate on scalar values. The directive allocates
- // accumulator storage and loads/stores values around recipe invocations.
- // By-reference recipes account for these effects in their bodies.
- effects.emplace_back(MemoryEffects::Allocate::get(), privateAccumulator);
- effects.emplace_back(MemoryEffects::Read::get(), &sourceVar);
- effects.emplace_back(MemoryEffects::Write::get(), privateAccumulator);
- effects.emplace_back(MemoryEffects::Read::get(), privateAccumulator);
- effects.emplace_back(MemoryEffects::Write::get(), &sourceVar);
- }
-
- auto recipe = SymbolTable::lookupNearestSymbolFrom<DeclareReductionOp>(
- op, cast<SymbolRefAttr>(recipeSym));
- if (!recipe) {
- continue;
- }
+ effects.emplace_back(MemoryEffects::Allocate::get(), priv);
+ effects.emplace_back(MemoryEffects::Free::get(), priv);
- SmallVector<std::pair<Value, RecipeEffectTarget>> mapping;
- populateRecipeEffectMapping(recipe, sourceVar, privateAccumulator, mapping);
- getRecipeEffects(recipe, mapping, effects);
+ effects.emplace_back(MemoryEffects::Read::get(), priv);
+ effects.emplace_back(MemoryEffects::Write::get(), priv);
}
}
@@ -3665,6 +3529,9 @@ void SectionsOp::getEffects(
getPrivatisationEffects(*this, effects);
getReductionEffects(*this, effects);
+ if (!getAssumeBoundedRecipeEffects() &&
+ (!getPrivateVars().empty() || !getReductionVars().empty()))
+ addUnknownMemoryEffects(effects);
}
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/OpenMP/memory-effects.mlir b/mlir/test/Dialect/OpenMP/memory-effects.mlir
index 72cd9dc916f692..8b4da709664580 100644
--- a/mlir/test/Dialect/OpenMP/memory-effects.mlir
+++ b/mlir/test/Dialect/OpenMP/memory-effects.mlir
@@ -31,14 +31,70 @@ func.func @sections_effects() {
return
}
-omp.private {type = private} @sections_p : memref<i32>
+func.func @sections_nowait_effects() {
+ // Without a reduction clause, nowait removes the implicit barrier effects.
+ // expected-remark at +1 {{operation has no memory effects}}
+ omp.sections nowait {
+ omp.section {
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
-func.func @sections_private_implicit_allocation(%x: memref<i32>) {
- // The directive allocates private storage even though the recipe is empty.
- // expected-remark at +1 {{found an instance of 'allocate' on block argument 0, on resource '<Default>'}}
- omp.sections nowait private(@sections_p %x -> %private_x : memref<i32>) {
+omp.private {type = firstprivate} @private_i32 : memref<i32> copy {
+^bb0(%original: memref<i32>, %private: memref<i32>):
+ // expected-remark at +1 {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
+ %v = memref.load %original[] : memref<i32>
+ // expected-remark at +1 {{found an instance of 'write' on op operand 1, on resource '<Default>'}}
+ memref.store %v, %private[] : memref<i32>
+ omp.yield(%private : memref<i32>)
+}
+omp.private {type = private} @private_ptr : !llvm.ptr
+
+func.func @sections_private_effects(%x: memref<i32>, %y: !llvm.ptr) {
+ // 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>'}}
+ // 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 op operand 1, on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'allocate' on block argument 1, on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'write' on block argument 1, on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'free' on block argument 1, on resource '<Default>'}}
+ omp.sections nowait private(@private_i32 %x -> %px,
+ @private_ptr %y -> %py : memref<i32>, !llvm.ptr) {
omp.section {
- ^bb0(%section_x: memref<i32>):
+ ^bb0(%sx: memref<i32>, %sy: !llvm.ptr):
+ omp.terminator
+ }
+ omp.terminator
+ } <{assume_bounded_recipe_effects}>
+ return
+}
+
+func.func private @custom_copy(memref<i32>, memref<i32>)
+
+omp.private {type = firstprivate} @custom_private_i32 : memref<i32> copy {
+^bb0(%original: memref<i32>, %private: memref<i32>):
+ func.call @custom_copy(%original, %private) : (memref<i32>, memref<i32>) -> ()
+ omp.yield(%private : memref<i32>)
+}
+
+func.func @sections_unmarked_private_effects(%x: memref<i32>) {
+ // Without an opt-in, the copy may access arbitrary memory. With nowait,
+ // these untargeted effects cannot come from an implicit barrier.
+ // 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>'}}
+ // 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 resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'write' on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'allocate' on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'free' on resource '<Default>'}}
+ omp.sections nowait private(@custom_private_i32 %x -> %px : memref<i32>) {
+ omp.section {
+ ^bb0(%sx: memref<i32>):
omp.terminator
}
omp.terminator
@@ -61,16 +117,16 @@ combiner {
}
func.func @sections_reduction_effects(%x: memref<f32>) {
- // The first two effects come from the implicit barrier, which nowait does
- // not currently eliminate when a reduction clause is present. Allocation,
- // initialization and combination access memory outside the by-value recipe.
- // 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 'read' on op operand 0, on resource '<Default>'}}
- // expected-remark at +3 {{found an instance of 'write' on block argument 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>'}}
+ // TODO: A nowait clause does not currently eliminate a barrier from a sections construct.
+ // The first two effects come from the implicit barrier.
+ // 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>'}}
+ // expected-remark at below {{found an instance of 'write' on block argument 0, 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 'write' on op operand 0, on resource '<Default>'}}
omp.sections nowait reduction(
@add_f32 %x -> %private_x : memref<f32>) {
omp.section {
@@ -78,100 +134,68 @@ func.func @sections_reduction_effects(%x: memref<f32>) {
omp.terminator
}
omp.terminator
- }
+ } <{assume_bounded_recipe_effects}>
return
}
-// Effects nested in the copy region must be traced through subviews to the
-// original mold and private arguments.
-omp.private {type = firstprivate} @sections_copy_subviews : memref<4xi32> copy {
-^bb0(%mold: memref<4xi32>, %private: memref<4xi32>):
- // Use scf.execute_region to test recursive effects.
- scf.execute_region {
- // expected-remark at +1 {{operation has no memory effects}}
- %c0 = arith.constant 0 : index
- // expected-remark at +1 {{operation has no memory effects}}
- %mold_view = memref.subview %mold[1] [2] [1] : memref<4xi32> to memref<2xi32, strided<[1], offset: 1>>
- // expected-remark at +1 {{operation has no memory effects}}
- %private_view = memref.subview %private[1] [2] [1] : memref<4xi32> to memref<2xi32, strided<[1], offset: 1>>
- // expected-remark at +1 {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
- %value = memref.load %mold_view[%c0] : memref<2xi32, strided<[1], offset: 1>>
- // expected-remark at +1 {{found an instance of 'write' on op operand 1, on resource '<Default>'}}
- memref.store %value, %private_view[%c0] : memref<2xi32, strided<[1], offset: 1>>
- scf.yield
- }
- omp.yield(%private : memref<4xi32>)
-} dealloc {
-^bb0(%private: memref<4xi32>):
- // expected-remark at +1 {{found an instance of 'free' on op operand 0, on resource '<Default>'}}
- memref.dealloc %private : memref<4xi32>
- omp.yield
-}
-
-func.func @sections_private_subview_effects(%x: memref<4xi32>) {
- // expected-remark at +4 {{found an instance of 'allocate' 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 'write' on block argument 0, on resource '<Default>'}}
- // expected-remark at +1 {{found an instance of 'free' on block argument 0, on resource '<Default>'}}
- omp.sections nowait private(@sections_copy_subviews %x -> %private_x : memref<4xi32>) {
+omp.declare_reduction @identity_f32 : f32
+init {
+^bb0(%mold: f32):
+ omp.yield(%mold : f32)
+}
+combiner {
+^bb0(%lhs: f32, %rhs: f32):
+ omp.yield(%rhs : f32)
+}
+
+func.func @sections_device_reduction_effects(%x: memref<f32>) {
+ // 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 'write' 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 'read' on block argument 0, on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'write' on block argument 0, on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'free' on block argument 0, on resource '<Default>'}}
+ omp.sections reduction(@identity_f32 %x -> %px : memref<f32>) {
omp.section {
- ^bb0(%section_x: memref<4xi32>):
+ ^bb0(%sx: memref<f32>):
omp.terminator
}
omp.terminator
- }
+ } <{assume_bounded_recipe_effects}>
return
}
-// Model allocation, initialisation, combination and cleanup effects.
-// The allocation is local to the recipe initialiser.
-// The combiner LHS can be the source or a private accumulator.
-omp.declare_reduction @sections_add_byref : memref<f32>
-alloc {
- // expected-remark at +1 {{found an instance of 'allocate' on op result 0, on resource '<Default>'}}
- %allocated = memref.alloc() : memref<f32>
- omp.yield(%allocated : memref<f32>)
-}
+
+func.func private @custom_combiner(f32, f32) -> f32
+
+omp.declare_reduction @custom_f32 : f32
init {
-^bb0(%mold: memref<f32>, %private: memref<f32>):
- // expected-remark at +1 {{operation has no memory effects}}
- %zero = arith.constant 0.0 : f32
- // expected-remark at +1 {{found an instance of 'write' on op operand 1, on resource '<Default>'}}
- memref.store %zero, %private[] : memref<f32>
- omp.yield(%private : memref<f32>)
+^bb0(%mold: f32):
+ omp.yield(%mold : f32)
}
combiner {
-^bb0(%lhs: memref<f32>, %rhs: memref<f32>):
- // expected-remark at +1 {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
- %left = memref.load %lhs[] : memref<f32>
- // expected-remark at +1 {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
- %right = memref.load %rhs[] : memref<f32>
- // expected-remark at +1 {{operation has no memory effects}}
- %sum = arith.addf %left, %right : f32
- // expected-remark at +1 {{found an instance of 'write' on op operand 1, on resource '<Default>'}}
- memref.store %sum, %lhs[] : memref<f32>
- omp.yield(%lhs : memref<f32>)
-}
-cleanup {
-^bb0(%private: memref<f32>):
- // expected-remark at +1 {{found an instance of 'free' on op operand 0, on resource '<Default>'}}
- memref.dealloc %private : memref<f32>
- omp.yield
+^bb0(%lhs: f32, %rhs: f32):
+ %v = func.call @custom_combiner(%lhs, %rhs) : (f32, f32) -> f32
+ omp.yield(%v : f32)
}
-func.func @sections_reduction_recipe_effects(%x: memref<f32>) {
- // The first two effects come from the implicit barrier.
- // expected-remark at +8 {{found an instance of 'read' on resource '<Default>'}}
- // expected-remark at +7 {{found an instance of 'write' on resource '<Default>'}}
- // expected-remark at +6 {{found an instance of 'allocate' on resource '<Default>'}}
- // expected-remark at +5 {{found an instance of 'write' on block argument 0, on resource '<Default>'}}
- // expected-remark at +4 {{found an instance of 'read' on op operand 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 'write' on op operand 0, on resource '<Default>'}}
- // expected-remark at +1 {{found an instance of 'free' on block argument 0, on resource '<Default>'}}
- omp.sections nowait reduction(byref @sections_add_byref %x -> %private_x : memref<f32>) {
+func.func @sections_custom_scalar_reduction(%x: memref<f32>) {
+ // A custom scalar reduction can access arbitrary memory through a call.
+ // 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>'}}
+ // expected-remark at below {{found an instance of 'write' on block argument 0, 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 'write' on op operand 0, on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'allocate' on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'free' on resource '<Default>'}}
+ omp.sections nowait reduction(@custom_f32 %x -> %p : memref<f32>) {
omp.section {
- ^bb0(%section_x: memref<f32>):
+ ^bb0(%s: memref<f32>):
omp.terminator
}
omp.terminator
@@ -179,31 +203,22 @@ func.func @sections_reduction_recipe_effects(%x: memref<f32>) {
return
}
-// Unknown effects inside a nested region require an untargeted read and write,
-// while retaining known effects already collected from the recipe.
-func.func private @unknown_recipe_effects(memref<i32>, i32)
-
-omp.private {type = firstprivate} @sections_unknown_copy : memref<i32> copy {
-^bb0(%mold: memref<i32>, %private: memref<i32>):
- // expected-remark at +1 {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
- %value = memref.load %mold[] : memref<i32>
- scf.execute_region {
- func.call @unknown_recipe_effects(%private, %value) : (memref<i32>, i32) -> ()
- scf.yield
- }
- omp.yield(%private : memref<i32>)
-}
-
-func.func @sections_private_unknown_recipe_effects(%x: memref<i32>) {
- // expected-remark at +6 {{found an instance of 'allocate' on block argument 0, on resource '<Default>'}}
- // expected-remark at +5 {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
- // expected-remark at +4 {{found an instance of 'read' on resource '<Default>'}}
- // expected-remark at +3 {{found an instance of 'write' on resource '<Default>'}}
- // expected-remark at +2 {{found an instance of 'allocate' on resource '<Default>'}}
- // expected-remark at +1 {{found an instance of 'free' on resource '<Default>'}}
- omp.sections nowait private(@sections_unknown_copy %x -> %private_x : memref<i32>) {
+func.func @sections_unmarked_builtin_reduction(%x: memref<f32>) {
+ // Even a simple recipe requires an explicit opt-in. The interface does not
+ // inspect recipe bodies to establish the bound.
+ // 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 resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'free' 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 'write' 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 'free' on block argument 0, on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'read' on block argument 0, on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'write' on block argument 0, on resource '<Default>'}}
+ omp.sections nowait reduction(@add_f32 %x -> %px : memref<f32>) {
omp.section {
- ^bb0(%section_x: memref<i32>):
+ ^bb0(%sx: memref<f32>):
omp.terminator
}
omp.terminator
>From 288e3ffbc7c48be974f84f91d582529e2a9cbc7b Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com" <jamie.rowlinson-patel at arm.com>
Date: Fri, 16 Oct 2026 14:58:24 +0000
Subject: [PATCH 07/17] Fix comments
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index dd6956e6cd728c..effef2cb0fd27e 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3571,6 +3571,7 @@ static bool isNonDerived(const semantics::Symbol &sym) {
static bool isNonDerivedItem(const semantics::Symbol *sym) {
const auto &ultimate = sym->GetUltimate();
+ // May be a common block
if (const auto *common =
ultimate.detailsIf<semantics::CommonBlockDetails>()) {
return llvm::all_of(common->objects(), [](const auto &member) {
@@ -3679,7 +3680,7 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
// SECTIONS construct.
auto sectionsOp = mlir::omp::SectionsOp::create(builder, loc, clauseOps);
- // Becuase Flang uses eager privatisation clausOps.privateVars is empty.
+ // Because Flang uses eager privatisation clausOps.privateVars is empty.
bool allPrivateVarsEligible =
llvm::all_of(dsp.getAllSymbolsToPrivatize(), isNonDerivedItem);
>From 13c64a00fbc55b685599fb3fec5704cb9f5256b5 Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com" <jamie.rowlinson-patel at arm.com>
Date: Fri, 16 Oct 2026 15:05:53 +0000
Subject: [PATCH 08/17] Update sections op contract
---
flang/test/Lower/OpenMP/sections.f90 | 1 -
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 9 +++------
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/flang/test/Lower/OpenMP/sections.f90 b/flang/test/Lower/OpenMP/sections.f90
index 10668c291996ff..08a69a56c42010 100644
--- a/flang/test/Lower/OpenMP/sections.f90
+++ b/flang/test/Lower/OpenMP/sections.f90
@@ -76,7 +76,6 @@ program sample
count = count * eta
double_count = count - eta
!$omp end sections
-
!$omp sections
call foo()
!$omp end sections nowait
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 6cad7e3fceafd7..2642c36541e789 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -358,14 +358,11 @@ def SectionsOp : OpenMP_Op<"sections", traits = [
- These effects apply only to storage directly designated by the original
variables and their corresponding private block arguments.
- They may not access other storage, including globals or pointees reached
- by loading pointers from these variables. Such accesses, or allocations
- and frees of other storage, are outside the assumed bounds.
+ by loading pointers from these variables.
These are upper bounds, not a claim that every recipe performs every
- permitted effect. The flag applies only to implicit recipe effects;
- explicit nested operations and barrier effects are modelled separately.
- Without the flag, referenced recipes are assumed to have arbitrary memory
- effects.
+ permitted effect. Without the flag, referenced recipes are assumed to have
+ arbitrary memory effects.
Flang currently sets it when all private and reduction variables are
non-derived. These checks do not establish that all recipe effects satisfy
>From 85826458be075bcef1d91fa57e257616338a0752 Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com" <jamie.rowlinson-patel at arm.com>
Date: Fri, 16 Oct 2026 15:27:37 +0000
Subject: [PATCH 09/17] Streamline test
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 15 ++--
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 9 +-
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 61 +++++++++-----
mlir/test/Dialect/OpenMP/memory-effects.mlir | 82 -------------------
4 files changed, 49 insertions(+), 118 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index effef2cb0fd27e..f0f6e6db0a29b2 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3569,8 +3569,8 @@ static bool isNonDerived(const semantics::Symbol &sym) {
return type && type->AsIntrinsic();
}
-static bool isNonDerivedItem(const semantics::Symbol *sym) {
- const auto &ultimate = sym->GetUltimate();
+static bool isNonDerivedItem(const semantics::Symbol &sym) {
+ const auto &ultimate = sym.GetUltimate();
// May be a common block
if (const auto *common =
ultimate.detailsIf<semantics::CommonBlockDetails>()) {
@@ -3583,8 +3583,7 @@ static bool isNonDerivedItem(const semantics::Symbol *sym) {
static bool isDerivedVariable(mlir::Value variable) {
mlir::Type type = variable.getType();
- return !fir::isa_derived(fir::getFortranElementType(type)) &&
- !fir::isa_volatile_type(type);
+ return !fir::isa_derived(fir::getFortranElementType(type));
}
static mlir::omp::SectionsOp
@@ -3680,9 +3679,11 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
// SECTIONS construct.
auto sectionsOp = mlir::omp::SectionsOp::create(builder, loc, clauseOps);
- // Because Flang uses eager privatisation clausOps.privateVars is empty.
- bool allPrivateVarsEligible =
- llvm::all_of(dsp.getAllSymbolsToPrivatize(), isNonDerivedItem);
+ // TODO: sections still uses eager privatisation. Use clauseOps.privateVars
+ // once delayedPrivatization is turned on.
+ bool allPrivateVarsEligible = llvm::all_of(
+ dsp.getAllSymbolsToPrivatize(),
+ [](const semantics::Symbol *sym) { return isNonDerivedItem(*sym); });
bool allReductionVarsEligible =
llvm::all_of(clauseOps.reductionVars, [lpAlloca](mlir::Value variable) {
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 2642c36541e789..126b9f0cbf9530 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -350,9 +350,8 @@ def SectionsOp : OpenMP_Op<"sections", traits = [
assume that all regions of the referenced privatisation and reduction
recipes obey the following bounds at this use:
- Privatisation may read the original variable as a mold or firstprivate
- copy source, and may allocate, write and free private storage. It
- may not write or free the original variable. Lastprivate copy-back is
- represented by explicit nested operations.
+ copy source, and may allocate, write and free private storage.
+ Lastprivate copy-back is represented by explicit nested operations.
- Reduction may read and write the original variable. It may allocate,
free, read or write private accumulators.
- These effects apply only to storage directly designated by the original
@@ -363,10 +362,6 @@ def SectionsOp : OpenMP_Op<"sections", traits = [
These are upper bounds, not a claim that every recipe performs every
permitted effect. Without the flag, referenced recipes are assumed to have
arbitrary memory effects.
-
- Flang currently sets it when all private and reduction variables are
- non-derived. These checks do not establish that all recipe effects satisfy
- the bounds: user-defined reductions may exceed them.
}] # clausesDescription;
let arguments = !con(clausesArgs, (ins
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 617a4d79b49416..63181c01c91679 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -3478,11 +3478,14 @@ static void addUnknownMemoryEffects(
/// Privatisation may read the original as a mold or firstprivate copy source.
/// Any lastprivate copy-back is explicit in the nested operations.
static void getPrivatisationEffects(
- SectionsOp op, SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
- auto iface = cast<BlockArgOpenMPOpInterface>(op.getOperation());
- for (auto [source, priv] : llvm::zip_equal(op.getPrivateVarsMutable(),
- iface.getPrivateBlockArgs())) {
+ Operation *op, SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
+ auto iface = dyn_cast<BlockArgOpenMPOpInterface>(op);
+
+ OperandRange sourceVars = iface.getPrivateVars();
+ for (OpOperand &source :
+ MutableArrayRef(sourceVars.getBase(), sourceVars.size()))
effects.emplace_back(MemoryEffects::Read::get(), &source);
+ for (BlockArgument priv : iface.getPrivateBlockArgs()) {
effects.emplace_back(MemoryEffects::Write::get(), priv);
effects.emplace_back(MemoryEffects::Allocate::get(), priv);
effects.emplace_back(MemoryEffects::Free::get(), priv);
@@ -3490,22 +3493,31 @@ static void getPrivatisationEffects(
}
/// Reductions initialise private accumulators and combine them into the
-/// original variables.
+/// original variables. Account for all three reduction clauses.
static void
-getReductionEffects(SectionsOp op,
+getReductionEffects(Operation *op,
SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
- auto iface = cast<BlockArgOpenMPOpInterface>(op.getOperation());
- for (auto [source, priv] : llvm::zip_equal(op.getReductionVarsMutable(),
- iface.getReductionBlockArgs())) {
- effects.emplace_back(MemoryEffects::Read::get(), &source);
- effects.emplace_back(MemoryEffects::Write::get(), &source);
+ auto iface = dyn_cast<BlockArgOpenMPOpInterface>(op);
+
+ auto addEffects = [&](OperandRange sourceVars,
+ ArrayRef<BlockArgument> privateArgs) {
+ for (OpOperand &source :
+ MutableArrayRef(sourceVars.getBase(), sourceVars.size())) {
+ effects.emplace_back(MemoryEffects::Read::get(), &source);
+ effects.emplace_back(MemoryEffects::Write::get(), &source);
+ }
- effects.emplace_back(MemoryEffects::Allocate::get(), priv);
- effects.emplace_back(MemoryEffects::Free::get(), priv);
+ for (BlockArgument priv : privateArgs) {
+ effects.emplace_back(MemoryEffects::Allocate::get(), priv);
+ effects.emplace_back(MemoryEffects::Free::get(), priv);
+ effects.emplace_back(MemoryEffects::Read::get(), priv);
+ effects.emplace_back(MemoryEffects::Write::get(), priv);
+ }
+ };
- effects.emplace_back(MemoryEffects::Read::get(), priv);
- effects.emplace_back(MemoryEffects::Write::get(), priv);
- }
+ addEffects(iface.getReductionVars(), iface.getReductionBlockArgs());
+ addEffects(iface.getInReductionVars(), iface.getInReductionBlockArgs());
+ addEffects(iface.getTaskReductionVars(), iface.getTaskReductionBlockArgs());
}
static void getBarrierEffects(
@@ -3526,12 +3538,17 @@ void SectionsOp::getEffects(
if (!getNowait() || (!getReductionVarsMutable().empty() && getNowait())) {
getBarrierEffects(effects);
}
-
- getPrivatisationEffects(*this, effects);
- getReductionEffects(*this, effects);
- if (!getAssumeBoundedRecipeEffects() &&
- (!getPrivateVars().empty() || !getReductionVars().empty()))
- addUnknownMemoryEffects(effects);
+ bool hasReduction = !getReductionVars().empty();
+ bool hasPrivatisation = !getPrivateVars().empty();
+ if (getAssumeBoundedRecipeEffects()) {
+ if (hasPrivatisation)
+ getPrivatisationEffects(*this, effects);
+ if (hasReduction)
+ getReductionEffects(*this, effects);
+ } else {
+ if (hasReduction || hasPrivatisation)
+ addUnknownMemoryEffects(effects);
+ }
}
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/OpenMP/memory-effects.mlir b/mlir/test/Dialect/OpenMP/memory-effects.mlir
index 8b4da709664580..de11f8768f46de 100644
--- a/mlir/test/Dialect/OpenMP/memory-effects.mlir
+++ b/mlir/test/Dialect/OpenMP/memory-effects.mlir
@@ -73,35 +73,6 @@ func.func @sections_private_effects(%x: memref<i32>, %y: !llvm.ptr) {
return
}
-func.func private @custom_copy(memref<i32>, memref<i32>)
-
-omp.private {type = firstprivate} @custom_private_i32 : memref<i32> copy {
-^bb0(%original: memref<i32>, %private: memref<i32>):
- func.call @custom_copy(%original, %private) : (memref<i32>, memref<i32>) -> ()
- omp.yield(%private : memref<i32>)
-}
-
-func.func @sections_unmarked_private_effects(%x: memref<i32>) {
- // Without an opt-in, the copy may access arbitrary memory. With nowait,
- // these untargeted effects cannot come from an implicit barrier.
- // 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>'}}
- // 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 resource '<Default>'}}
- // expected-remark at below {{found an instance of 'write' on resource '<Default>'}}
- // expected-remark at below {{found an instance of 'allocate' on resource '<Default>'}}
- // expected-remark at below {{found an instance of 'free' on resource '<Default>'}}
- omp.sections nowait private(@custom_private_i32 %x -> %px : memref<i32>) {
- omp.section {
- ^bb0(%sx: memref<i32>):
- omp.terminator
- }
- omp.terminator
- }
- return
-}
-
omp.declare_reduction @add_f32 : f32
init {
^bb0(%unused: f32):
@@ -138,36 +109,6 @@ func.func @sections_reduction_effects(%x: memref<f32>) {
return
}
-omp.declare_reduction @identity_f32 : f32
-init {
-^bb0(%mold: f32):
- omp.yield(%mold : f32)
-}
-combiner {
-^bb0(%lhs: f32, %rhs: f32):
- omp.yield(%rhs : f32)
-}
-
-func.func @sections_device_reduction_effects(%x: memref<f32>) {
- // 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 'write' 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 'read' on block argument 0, on resource '<Default>'}}
- // expected-remark at below {{found an instance of 'write' on block argument 0, on resource '<Default>'}}
- // expected-remark at below {{found an instance of 'free' on block argument 0, on resource '<Default>'}}
- omp.sections reduction(@identity_f32 %x -> %px : memref<f32>) {
- omp.section {
- ^bb0(%sx: memref<f32>):
- omp.terminator
- }
- omp.terminator
- } <{assume_bounded_recipe_effects}>
- return
-}
-
-
func.func private @custom_combiner(f32, f32) -> f32
omp.declare_reduction @custom_f32 : f32
@@ -202,26 +143,3 @@ func.func @sections_custom_scalar_reduction(%x: memref<f32>) {
}
return
}
-
-func.func @sections_unmarked_builtin_reduction(%x: memref<f32>) {
- // Even a simple recipe requires an explicit opt-in. The interface does not
- // inspect recipe bodies to establish the bound.
- // 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 resource '<Default>'}}
- // expected-remark at below {{found an instance of 'free' 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 'write' 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 'free' on block argument 0, on resource '<Default>'}}
- // expected-remark at below {{found an instance of 'read' on block argument 0, on resource '<Default>'}}
- // expected-remark at below {{found an instance of 'write' on block argument 0, on resource '<Default>'}}
- omp.sections nowait reduction(@add_f32 %x -> %px : memref<f32>) {
- omp.section {
- ^bb0(%sx: memref<f32>):
- omp.terminator
- }
- omp.terminator
- }
- return
-}
>From 31a8ec58cb5c6ae1de606ad0f846b7147b77969b Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com" <jamie.rowlinson-patel at arm.com>
Date: Mon, 19 Oct 2026 10:16:49 +0000
Subject: [PATCH 10/17] Fix memory-effects.mlir test failure
---
mlir/test/Dialect/OpenMP/memory-effects.mlir | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/mlir/test/Dialect/OpenMP/memory-effects.mlir b/mlir/test/Dialect/OpenMP/memory-effects.mlir
index de11f8768f46de..46427014f6d8a1 100644
--- a/mlir/test/Dialect/OpenMP/memory-effects.mlir
+++ b/mlir/test/Dialect/OpenMP/memory-effects.mlir
@@ -124,14 +124,9 @@ combiner {
func.func @sections_custom_scalar_reduction(%x: memref<f32>) {
// A custom scalar reduction can access arbitrary memory through a call.
+ // Without assume_bounded_recipe_effects, the recipe effects are unspecified.
// 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>'}}
- // expected-remark at below {{found an instance of 'write' on block argument 0, 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 'write' on op operand 0, on resource '<Default>'}}
// expected-remark at below {{found an instance of 'allocate' on resource '<Default>'}}
// expected-remark at below {{found an instance of 'free' on resource '<Default>'}}
omp.sections nowait reduction(@custom_f32 %x -> %p : memref<f32>) {
>From 90b3449ceea3e315059d222375306e76c19f125e Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com" <jamie.rowlinson-patel at arm.com>
Date: Tue, 20 Oct 2026 10:04:03 +0000
Subject: [PATCH 11/17] Move bounded recipe attribute to clauses from
constructs
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 42 +++---
.../Lower/OpenMP/recipe-memory-effects.f90 | 131 +++++++++++++-----
.../mlir/Dialect/OpenMP/OpenMPClauses.td | 44 +++++-
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 26 +---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 28 ++--
mlir/test/Dialect/OpenMP/memory-effects.mlir | 102 +++++++++-----
6 files changed, 235 insertions(+), 138 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index f0f6e6db0a29b2..d95679c24139ec 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3564,26 +3564,21 @@ findEnclosingParallelOp(fir::FirOpBuilder &builder) {
return {};
}
-static bool isNonDerived(const semantics::Symbol &sym) {
- const auto *type = sym.GetUltimate().GetType();
- return type && type->AsIntrinsic();
+static bool isNonDerivedNonAllocatable(const semantics::Symbol &sym) {
+ const auto &ultimate = sym.GetUltimate();
+ const auto *type = ultimate.GetType();
+ return type && type->AsIntrinsic() && !semantics::IsAllocatable(ultimate);
}
-static bool isNonDerivedItem(const semantics::Symbol &sym) {
+static bool isNonDerivedNonAllocatableItem(const semantics::Symbol &sym) {
const auto &ultimate = sym.GetUltimate();
- // May be a common block
if (const auto *common =
ultimate.detailsIf<semantics::CommonBlockDetails>()) {
return llvm::all_of(common->objects(), [](const auto &member) {
- return isNonDerived(*member);
+ return isNonDerivedNonAllocatable(*member);
});
}
- return isNonDerived(ultimate);
-}
-
-static bool isDerivedVariable(mlir::Value variable) {
- mlir::Type type = variable.getType();
- return !fir::isa_derived(fir::getFortranElementType(type));
+ return isNonDerivedNonAllocatable(ultimate);
}
static mlir::omp::SectionsOp
@@ -3681,21 +3676,24 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
auto sectionsOp = mlir::omp::SectionsOp::create(builder, loc, clauseOps);
// TODO: sections still uses eager privatisation. Use clauseOps.privateVars
// once delayedPrivatization is turned on.
- bool allPrivateVarsEligible = llvm::all_of(
- dsp.getAllSymbolsToPrivatize(),
- [](const semantics::Symbol *sym) { return isNonDerivedItem(*sym); });
-
- bool allReductionVarsEligible =
+ const auto &privateSyms = dsp.getAllSymbolsToPrivatize();
+ if (!privateSyms.empty() &&
+ llvm::all_of(privateSyms, [](const semantics::Symbol *sym) {
+ return isNonDerivedNonAllocatableItem(*sym);
+ }))
+ sectionsOp.setPrivateAssumeBoundedRecipeEffects(true);
+
+ if (!clauseOps.reductionVars.empty() &&
llvm::all_of(clauseOps.reductionVars, [lpAlloca](mlir::Value variable) {
// The generated conditional-lastprivate recipe only accesses the
// intrinsic value and index fields of its accumulator, despite its
// record type.
return variable == lpAlloca ||
- isDerivedVariable(variable);
- });
-
- if (allPrivateVarsEligible && allReductionVarsEligible)
- sectionsOp.setAssumeBoundedRecipeEffects(true);
+ (!fir::isAllocatableType(variable.getType()) &&
+ !fir::isa_derived(
+ fir::getFortranElementType(variable.getType())));
+ }))
+ sectionsOp.setReductionAssumeBoundedRecipeEffects(true);
// Create entry block with reduction variables as arguments.
ObjectEntryBlockArgs args;
diff --git a/flang/test/Lower/OpenMP/recipe-memory-effects.f90 b/flang/test/Lower/OpenMP/recipe-memory-effects.f90
index 511a31adf2221a..631906ee7b45ba 100644
--- a/flang/test/Lower/OpenMP/recipe-memory-effects.f90
+++ b/flang/test/Lower/OpenMP/recipe-memory-effects.f90
@@ -1,8 +1,7 @@
-! RUN: bbc -emit-hlfir -fopenmp -fopenmp-version=50 %s -o - | FileCheck %s
! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %s -o - | FileCheck %s
-! The bounded-recipe-effects assumption is selected using the types of all
-! reduction and privatized variables.
+! Each clause has its own bounded-recipe-effects assumption, which requires
+! all variables in that clause to be non-derived and non-allocatable.
module recipe_memory_effects
implicit none
type :: record
@@ -10,10 +9,10 @@ module recipe_memory_effects
integer :: value
end type
contains
- ! Scalar and boxed intrinsic reductions and privatization are eligible.
+ ! Intrinsic private and reduction variables are eligible independently.
! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPintrinsic_clauses(
! CHECK: omp.sections reduction(
- ! CHECK: } <assume_bounded_recipe_effects>
+ ! CHECK: } <private_assume_bounded_recipe_effects, reduction_assume_bounded_recipe_effects>
subroutine intrinsic_clauses(x, y, p, f, l)
integer :: x, p, f, l
real :: y
@@ -25,7 +24,63 @@ subroutine intrinsic_clauses(x, y, p, f, l)
!$omp end sections
end subroutine
- ! A derived private variable prevents the assumption.
+ ! Non-derived private, firstprivate and lastprivate variables are eligible
+ ! without a reduction clause.
+ ! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPintrinsic_private(
+ ! CHECK: omp.sections {
+ ! CHECK: } <private_assume_bounded_recipe_effects>
+ ! CHECK: return
+ subroutine intrinsic_private(p, f, l)
+ integer :: p, f, l
+ !$omp sections private(p) firstprivate(f) lastprivate(l)
+ p = f
+ l = p
+ !$omp end sections
+ end subroutine
+
+ ! An allocatable variable prevents the assumption for the whole private clause.
+ ! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPboxed_private(
+ ! CHECK: omp.sections {
+ ! CHECK-NOT: assume_bounded_recipe_effects
+ ! CHECK: return
+ subroutine boxed_private(a, p)
+ real, allocatable :: a(:)
+ real, pointer :: p
+ !$omp sections private(a, p)
+ a = 1.0
+ nullify(p)
+ !$omp end sections
+ end subroutine
+
+ ! Scalar allocatables are also excluded, independently of the other clause.
+ ! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPallocatable_private_scalar_reduction(
+ ! CHECK: omp.sections reduction(
+ ! CHECK-NOT: private_assume_bounded_recipe_effects
+ ! CHECK: } <reduction_assume_bounded_recipe_effects>
+ ! CHECK: return
+ subroutine allocatable_private_scalar_reduction(a, x)
+ real, allocatable :: a
+ integer :: x
+ !$omp sections private(a) reduction(+:x)
+ x = x + 1
+ !$omp end sections
+ end subroutine
+
+ ! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPallocatable_reduction_scalar_private(
+ ! CHECK: omp.sections reduction(
+ ! CHECK-NOT: reduction_assume_bounded_recipe_effects
+ ! CHECK: } <private_assume_bounded_recipe_effects>
+ ! CHECK: return
+ subroutine allocatable_reduction_scalar_private(a, x)
+ real, allocatable :: a
+ integer :: x
+ !$omp sections reduction(+:a) private(x)
+ x = 1
+ a = a + x
+ !$omp end sections
+ end subroutine
+
+ ! A derived private variable prevents the assumption for the private clause.
! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPderived_private(
! CHECK: omp.sections {
! CHECK-NOT: assume_bounded_recipe_effects
@@ -39,22 +94,10 @@ subroutine derived_private()
!$omp end sections
end subroutine
- ! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPintrinsic_private(
- ! CHECK: omp.sections {
- ! CHECK: } <assume_bounded_recipe_effects>
- subroutine intrinsic_private()
- integer :: x
- real :: y
- !$omp sections private(x, y)
- x = 1
- y = 1.0
- !$omp end sections
- end subroutine
-
! The generated conditional-lastprivate record is eligible.
! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPconditional_lastprivate(
! CHECK: omp.sections reduction(
- ! CHECK: } <assume_bounded_recipe_effects>
+ ! CHECK: } <reduction_assume_bounded_recipe_effects>
subroutine conditional_lastprivate(x, y)
integer :: x, y
!$omp sections lastprivate(conditional:x) firstprivate(x) reduction(+:y)
@@ -63,38 +106,56 @@ subroutine conditional_lastprivate(x, y)
!$omp end sections
end subroutine
- ! A user-derived reduction still prevents the assumption.
+ ! A derived reduction prevents the assumption for the whole reduction clause,
+ ! independently of the non-derived private variables.
! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPconditional_lastprivate_derived_reduction(
- ! CHECK: omp.sections reduction(byref
- ! CHECK-NOT: assume_bounded_recipe_effects
+ ! CHECK: omp.sections reduction({{(byref )?}}@
+ ! CHECK-NOT: reduction_assume_bounded_recipe_effects
+ ! CHECK: } <private_assume_bounded_recipe_effects>
+ ! CHECK-NOT: reduction_assume_bounded_recipe_effects
! CHECK: return
subroutine conditional_lastprivate_derived_reduction(x, z)
- integer :: x
+ integer :: x, p
type(record) :: z
!$omp declare reduction(record_sum : record : &
!$omp & omp_out%value = omp_out%value + omp_in%value) &
!$omp & initializer(omp_priv%value = 0)
!$omp parallel
- !$omp sections lastprivate(conditional:x) reduction(record_sum:z)
- x = 1
+ !$omp sections lastprivate(conditional:x) reduction(record_sum:z) private(p)
+ p = 1
+ x = p
z%value = z%value + x
!$omp end sections
!$omp end parallel
end subroutine
- ! A derived private variable still prevents the assumption.
- ! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPconditional_lastprivate_derived_private(
- ! CHECK: omp.sections reduction(byref
+ ! A common block is eligible when all its members are non-derived.
+ ! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPintrinsic_common_private(
+ ! CHECK: omp.sections {
+ ! CHECK: } <private_assume_bounded_recipe_effects>
+ ! CHECK: return
+ subroutine intrinsic_common_private()
+ integer :: x
+ real :: y
+ common /intrinsic_block/ x, y
+ !$omp sections private(/intrinsic_block/)
+ x = 1
+ y = 1.0
+ !$omp end sections
+ end subroutine
+
+ ! A derived common-block member prevents the private-clause assumption.
+ ! CHECK-LABEL: func.func @_QMrecipe_memory_effectsPderived_common_private(
+ ! CHECK: omp.sections {
! CHECK-NOT: assume_bounded_recipe_effects
! CHECK: return
- subroutine conditional_lastprivate_derived_private(x, z)
+ subroutine derived_common_private()
integer :: x
type(record) :: z
- !$omp parallel
- !$omp sections lastprivate(conditional:x) private(z)
- z%value = 1
- x = z%value
- !$omp end sections
- !$omp end parallel
+ common /derived_block/ x, z
+ !$omp sections private(/derived_block/)
+ x = 1
+ z%value = x
+ !$omp end sections
end subroutine
end module
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td
index 81419ac4b9920b..ab6e7178c9f2e9 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td
@@ -1514,22 +1514,37 @@ def OpenMP_PriorityClause : OpenMP_PriorityClauseSkip<>;
class OpenMP_PrivateClauseSkip<
bit traits = false, bit arguments = false, bit assemblyFormat = false,
- bit description = false, bit extraClassDeclaration = false
+ bit description = false, bit extraClassDeclaration = false,
+ // Only sections currently consumes the recipe-effect assumption.
+ bit boundedRecipeEffects = false
> : OpenMP_Clause<traits, arguments, assemblyFormat, description,
extraClassDeclaration> {
let traits = [
BlockArgOpenMPOpInterface
];
- let arguments = (ins
+ let arguments = !con((ins
Variadic<AnyType>:$private_vars,
OptionalAttr<SymbolRefArrayAttr>:$private_syms,
// Set this attribute if a barrier is needed after initialization and
// copying of lastprivate variables.
UnitAttr:$private_needs_barrier
- );
+ ), !if(boundedRecipeEffects, (ins
+ UnitAttr:$private_assume_bounded_recipe_effects
+ ), (ins)));
- // TODO: Add description.
+ let description = [{
+ The optional `private_assume_bounded_recipe_effects` unit attribute asserts
+ that all regions of every referenced privatisation recipe may only read the
+ original variable and allocate, write or free private storage.
+ Lastprivate copy-back is represented by explicit nested operations.
+
+ These effects apply only to storage directly designated by the original
+ variables and their corresponding private block arguments. Recipes may not
+ access other storage, including globals or pointees reached by loading
+ pointers from these variables. Without the attribute, referenced
+ privatisation recipes are assumed to have arbitrary memory effects.
+ }];
// Assembly format not defined because this clause must be processed together
// with the first region of the operation, as it defines entry block
// arguments.
@@ -1568,19 +1583,23 @@ def OpenMP_ProcBindClause : OpenMP_ProcBindClauseSkip<>;
class OpenMP_ReductionClauseSkip<
bit traits = false, bit arguments = false, bit assemblyFormat = false,
- bit description = false, bit extraClassDeclaration = false
+ bit description = false, bit extraClassDeclaration = false,
+ // Only sections currently consumes the recipe-effect assumption.
+ bit boundedRecipeEffects = false
> : OpenMP_Clause<traits, arguments, assemblyFormat, description,
extraClassDeclaration> {
let traits = [
BlockArgOpenMPOpInterface
];
- let arguments = (ins
+ let arguments = !con((ins
OptionalAttr<ReductionModifierAttr>:$reduction_mod,
Variadic<OpenMP_PointerLikeType>:$reduction_vars,
OptionalAttr<DenseBoolArrayAttr>:$reduction_byref,
OptionalAttr<SymbolRefArrayAttr>:$reduction_syms
- );
+ ), !if(boundedRecipeEffects, (ins
+ UnitAttr:$reduction_assume_bounded_recipe_effects
+ ), (ins)));
let extraClassDeclaration = [{
/// Returns the number of reduction variables.
@@ -1601,6 +1620,17 @@ class OpenMP_ReductionClauseSkip<
specifies how to combine the values from each iteration, section, team,
thread or simd lane defined by the operation's region into the final value,
which is available in the accumulator after they all complete.
+
+ The optional `reduction_assume_bounded_recipe_effects` unit attribute
+ asserts that all regions of every referenced reduction recipe may only read
+ or write the original variable and allocate, free, read or write private
+ accumulators.
+
+ These effects apply only to storage directly designated by the original
+ variables and their corresponding private block arguments. Recipes may not
+ access other storage, including globals or pointees reached by loading
+ pointers from these variables. Without the attribute, referenced reduction
+ recipes are assumed to have arbitrary memory effects.
}];
// Assembly format not defined because this clause must be processed together
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 126b9f0cbf9530..d08b27b90ede5c 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -333,8 +333,9 @@ def SectionsOp : OpenMP_Op<"sections", traits = [
HasParallelRegion,
DeclareOpInterfaceMethods<MemoryEffectsOpInterface>, RecursiveMemoryEffects
], clauses = [
- OpenMP_AllocateClause, OpenMP_NowaitClause, OpenMP_PrivateClause,
- OpenMP_ReductionClause
+ OpenMP_AllocateClause, OpenMP_NowaitClause,
+ OpenMP_PrivateClauseSkip<boundedRecipeEffects = true>,
+ OpenMP_ReductionClauseSkip<boundedRecipeEffects = true>
], singleRegion = true> {
let summary = "sections construct";
let description = [{
@@ -345,29 +346,8 @@ def SectionsOp : OpenMP_Op<"sections", traits = [
implicit task.
Block arguments for reduction variables should be mirrored in enclosed
`omp.section` operations.
-
- The `assume_bounded_recipe_effects` flag makes the memory-effect interface
- assume that all regions of the referenced privatisation and reduction
- recipes obey the following bounds at this use:
- - Privatisation may read the original variable as a mold or firstprivate
- copy source, and may allocate, write and free private storage.
- Lastprivate copy-back is represented by explicit nested operations.
- - Reduction may read and write the original variable. It may allocate,
- free, read or write private accumulators.
- - These effects apply only to storage directly designated by the original
- variables and their corresponding private block arguments.
- - They may not access other storage, including globals or pointees reached
- by loading pointers from these variables.
-
- These are upper bounds, not a claim that every recipe performs every
- permitted effect. Without the flag, referenced recipes are assumed to have
- arbitrary memory effects.
}] # clausesDescription;
- let arguments = !con(clausesArgs, (ins
- UnitAttr:$assume_bounded_recipe_effects
- ));
-
// Override region definition.
let regions = (region SizedRegion<1>:$region);
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 63181c01c91679..7acc0c45ed78f7 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -3429,16 +3429,17 @@ OperandRange SectionOp::getReductionVars() {
void SectionsOp::build(OpBuilder &builder, OperationState &state,
const SectionsOperands &clauses) {
MLIRContext *ctx = builder.getContext();
- // TODO Store clauses in op: privateVars, privateSyms, privateNeedsBarrier
SectionsOp::build(builder, state, clauses.allocateVars, clauses.allocatorVars,
makeDenseI64ArrayAttr(ctx, clauses.allocateAlignments),
makeDenseI64ArrayAttr(ctx, clauses.allocatePrivateIndices),
- clauses.nowait, /*private_vars=*/{},
- /*private_syms=*/nullptr, /*private_needs_barrier=*/nullptr,
+ clauses.nowait, clauses.privateVars,
+ makeArrayAttr(ctx, clauses.privateSyms),
+ clauses.privateNeedsBarrier,
+ /*private_assume_bounded_recipe_effects=*/nullptr,
clauses.reductionMod, clauses.reductionVars,
makeDenseBoolArrayAttr(ctx, clauses.reductionByref),
makeArrayAttr(ctx, clauses.reductionSyms),
- /*assume_bounded_recipe_effects=*/nullptr);
+ /*reduction_assume_bounded_recipe_effects=*/nullptr);
}
LogicalResult SectionsOp::verify() {
@@ -3538,16 +3539,17 @@ void SectionsOp::getEffects(
if (!getNowait() || (!getReductionVarsMutable().empty() && getNowait())) {
getBarrierEffects(effects);
}
- bool hasReduction = !getReductionVars().empty();
- bool hasPrivatisation = !getPrivateVars().empty();
- if (getAssumeBoundedRecipeEffects()) {
- if (hasPrivatisation)
- getPrivatisationEffects(*this, effects);
- if (hasReduction)
- getReductionEffects(*this, effects);
+
+ bool hasUnknownRecipeEffects =
+ (!getPrivateVars().empty() && !getPrivateAssumeBoundedRecipeEffects()) ||
+ (!getReductionVars().empty() &&
+ !getReductionAssumeBoundedRecipeEffects());
+
+ if (hasUnknownRecipeEffects) {
+ addUnknownMemoryEffects(effects);
} else {
- if (hasReduction || hasPrivatisation)
- addUnknownMemoryEffects(effects);
+ getPrivatisationEffects(*this, effects);
+ getReductionEffects(*this, effects);
}
}
diff --git a/mlir/test/Dialect/OpenMP/memory-effects.mlir b/mlir/test/Dialect/OpenMP/memory-effects.mlir
index 46427014f6d8a1..85927a3b36a22d 100644
--- a/mlir/test/Dialect/OpenMP/memory-effects.mlir
+++ b/mlir/test/Dialect/OpenMP/memory-effects.mlir
@@ -23,9 +23,6 @@ func.func @sections_effects() {
// 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
@@ -35,9 +32,6 @@ func.func @sections_nowait_effects() {
// Without a reduction clause, nowait removes the implicit barrier effects.
// expected-remark at +1 {{operation has no memory effects}}
omp.sections nowait {
- omp.section {
- omp.terminator
- }
omp.terminator
}
return
@@ -64,12 +58,20 @@ func.func @sections_private_effects(%x: memref<i32>, %y: !llvm.ptr) {
// expected-remark at below {{found an instance of 'free' on block argument 1, on resource '<Default>'}}
omp.sections nowait private(@private_i32 %x -> %px,
@private_ptr %y -> %py : memref<i32>, !llvm.ptr) {
- omp.section {
- ^bb0(%sx: memref<i32>, %sy: !llvm.ptr):
- omp.terminator
- }
omp.terminator
- } <{assume_bounded_recipe_effects}>
+ } <{private_assume_bounded_recipe_effects}>
+ return
+}
+
+func.func @sections_unbounded_private_effects(%x: memref<i32>) {
+ // Without the flag, all private recipe effects are arbitrary.
+ // 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 resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'free' on resource '<Default>'}}
+ omp.sections nowait private(@private_i32 %x -> %px : memref<i32>) {
+ omp.terminator
+ }
return
}
@@ -88,7 +90,7 @@ combiner {
}
func.func @sections_reduction_effects(%x: memref<f32>) {
- // TODO: A nowait clause does not currently eliminate a barrier from a sections construct.
+ // TODO: nowait does not currently eliminate the barrier with reductions.
// The first two effects come from the implicit barrier.
// expected-remark at below {{found an instance of 'read' on resource '<Default>'}}
// expected-remark at below {{found an instance of 'write' on resource '<Default>'}}
@@ -98,43 +100,67 @@ func.func @sections_reduction_effects(%x: memref<f32>) {
// expected-remark at below {{found an instance of 'write' on block argument 0, 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 '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.sections nowait reduction(@add_f32 %x -> %private_x : memref<f32>) {
omp.terminator
- } <{assume_bounded_recipe_effects}>
+ } <{reduction_assume_bounded_recipe_effects}>
return
}
-func.func private @custom_combiner(f32, f32) -> f32
-
-omp.declare_reduction @custom_f32 : f32
-init {
-^bb0(%mold: f32):
- omp.yield(%mold : f32)
+func.func @sections_unbounded_reduction_effects(%x: memref<f32>) {
+ // Without the flag, even a pure recipe is modeled with arbitrary 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 resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'free' on resource '<Default>'}}
+ omp.sections nowait reduction(@add_f32 %x -> %p : memref<f32>) {
+ omp.terminator
+ }
+ return
}
-combiner {
-^bb0(%lhs: f32, %rhs: f32):
- %v = func.call @custom_combiner(%lhs, %rhs) : (f32, f32) -> f32
- omp.yield(%v : f32)
+
+func.func @sections_bounded_private_unbounded_reduction(%x: memref<i32>, %y: memref<f32>) {
+ // A bounded private clause does not constrain an unbounded reduction clause.
+ // 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 resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'free' on resource '<Default>'}}
+ omp.sections nowait private(@private_i32 %x -> %px : memref<i32>)
+ reduction(@add_f32 %y -> %py : memref<f32>) {
+ omp.terminator
+ } <{private_assume_bounded_recipe_effects}>
+ return
}
-func.func @sections_custom_scalar_reduction(%x: memref<f32>) {
- // A custom scalar reduction can access arbitrary memory through a call.
- // Without assume_bounded_recipe_effects, the recipe effects are unspecified.
+func.func @sections_unbounded_private_bounded_reduction(%x: memref<i32>, %y: memref<f32>) {
+ // A bounded reduction clause does not constrain an unbounded private clause.
// 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 resource '<Default>'}}
// expected-remark at below {{found an instance of 'free' on resource '<Default>'}}
- omp.sections nowait reduction(@custom_f32 %x -> %p : memref<f32>) {
- omp.section {
- ^bb0(%s: memref<f32>):
- omp.terminator
- }
+ omp.sections nowait private(@private_i32 %x -> %px : memref<i32>)
+ reduction(@add_f32 %y -> %py : memref<f32>) {
omp.terminator
- }
+ } <{reduction_assume_bounded_recipe_effects}>
+ return
+}
+
+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 {{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>'}}
+ // 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 op operand 1, on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'write' on op operand 1, on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'allocate' on block argument 1, on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'free' on block argument 1, on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'read' on block argument 1, on resource '<Default>'}}
+ // expected-remark at below {{found an instance of 'write' on block argument 1, on resource '<Default>'}}
+ omp.sections nowait private(@private_i32 %x -> %px : memref<i32>)
+ reduction(@add_f32 %y -> %py : memref<f32>) {
+ omp.terminator
+ } <{private_assume_bounded_recipe_effects, reduction_assume_bounded_recipe_effects}>
return
}
>From 27ef5f8c19c37769d3c9daee4c6a5f93dfb34bb0 Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com" <jamie.rowlinson-patel at arm.com>
Date: Tue, 20 Oct 2026 11:48:36 +0000
Subject: [PATCH 12/17] Revert uncecessary changes
---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 7acc0c45ed78f7..bc511ad1629911 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -3432,9 +3432,8 @@ void SectionsOp::build(OpBuilder &builder, OperationState &state,
SectionsOp::build(builder, state, clauses.allocateVars, clauses.allocatorVars,
makeDenseI64ArrayAttr(ctx, clauses.allocateAlignments),
makeDenseI64ArrayAttr(ctx, clauses.allocatePrivateIndices),
- clauses.nowait, clauses.privateVars,
- makeArrayAttr(ctx, clauses.privateSyms),
- clauses.privateNeedsBarrier,
+ clauses.nowait, /*private_vars=*/{},
+ /*private_syms=*/nullptr, /*private_needs_barrier=*/nullptr,
/*private_assume_bounded_recipe_effects=*/nullptr,
clauses.reductionMod, clauses.reductionVars,
makeDenseBoolArrayAttr(ctx, clauses.reductionByref),
>From 8dcd0a736784ac1d286d54a776725249f75aafb5 Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com" <jamie.rowlinson-patel at arm.com>
Date: Tue, 20 Oct 2026 11:50:47 +0000
Subject: [PATCH 13/17] restore todo
---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index bc511ad1629911..1c869ad04bbcf6 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -3429,6 +3429,7 @@ OperandRange SectionOp::getReductionVars() {
void SectionsOp::build(OpBuilder &builder, OperationState &state,
const SectionsOperands &clauses) {
MLIRContext *ctx = builder.getContext();
+ // TODO Store clauses in op: privateVars, privateSyms, privateNeedsBarrier
SectionsOp::build(builder, state, clauses.allocateVars, clauses.allocatorVars,
makeDenseI64ArrayAttr(ctx, clauses.allocateAlignments),
makeDenseI64ArrayAttr(ctx, clauses.allocatePrivateIndices),
>From 453a6e8c44d565cd6eb961f444fe61cd542d6637 Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com" <jamie.rowlinson-patel at arm.com>
Date: Tue, 20 Oct 2026 12:08:48 +0000
Subject: [PATCH 14/17] Streamline tests
---
mlir/test/Dialect/OpenMP/memory-effects.mlir | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/mlir/test/Dialect/OpenMP/memory-effects.mlir b/mlir/test/Dialect/OpenMP/memory-effects.mlir
index 85927a3b36a22d..3110627a89b106 100644
--- a/mlir/test/Dialect/OpenMP/memory-effects.mlir
+++ b/mlir/test/Dialect/OpenMP/memory-effects.mlir
@@ -29,7 +29,7 @@ func.func @sections_effects() {
}
func.func @sections_nowait_effects() {
- // Without a reduction clause, nowait removes the implicit barrier effects.
+ // Nowait removes the implicit barrier effects.
// expected-remark at +1 {{operation has no memory effects}}
omp.sections nowait {
omp.terminator
@@ -47,7 +47,7 @@ omp.private {type = firstprivate} @private_i32 : memref<i32> copy {
}
omp.private {type = private} @private_ptr : !llvm.ptr
-func.func @sections_private_effects(%x: memref<i32>, %y: !llvm.ptr) {
+func.func @sections_bounded_private_effects(%x: memref<i32>, %y: !llvm.ptr) {
// 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>'}}
@@ -89,7 +89,7 @@ combiner {
omp.yield(%sum : f32)
}
-func.func @sections_reduction_effects(%x: memref<f32>) {
+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 {{found an instance of 'read' on resource '<Default>'}}
>From e458f40a05d0f6e75762692bed0860760b9c494b Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com" <jamie.rowlinson-patel at arm.com>
Date: Tue, 20 Oct 2026 13:59:19 +0000
Subject: [PATCH 15/17] Improve test comments. Exclude pointers and
allocatables from being labelled with the restricted recipe attribute
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index d95679c24139ec..6ba7da04c7d7c9 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3564,21 +3564,27 @@ findEnclosingParallelOp(fir::FirOpBuilder &builder) {
return {};
}
-static bool isNonDerivedNonAllocatable(const semantics::Symbol &sym) {
+static bool isNonDerivedNonPointer(const semantics::Symbol &sym) {
const auto &ultimate = sym.GetUltimate();
const auto *type = ultimate.GetType();
- return type && type->AsIntrinsic() && !semantics::IsAllocatable(ultimate);
+ return type && type->AsIntrinsic() && !semantics::IsAllocatable(ultimate) &&
+ !semantics::IsPointer(ultimate);
}
-static bool isNonDerivedNonAllocatableItem(const semantics::Symbol &sym) {
+static bool isNonDerivedNonPointer(const mlir::Value &variable) {
+ return !fir::isAllocatableType(variable.getType()) &&
+ !fir::isa_derived(fir::getFortranElementType(variable.getType()));
+}
+
+static bool isNonDerivedNonPointerItem(const semantics::Symbol &sym) {
const auto &ultimate = sym.GetUltimate();
if (const auto *common =
ultimate.detailsIf<semantics::CommonBlockDetails>()) {
return llvm::all_of(common->objects(), [](const auto &member) {
- return isNonDerivedNonAllocatable(*member);
+ return isNonDerivedNonPointer(*member);
});
}
- return isNonDerivedNonAllocatable(ultimate);
+ return isNonDerivedNonPointer(ultimate);
}
static mlir::omp::SectionsOp
@@ -3679,7 +3685,7 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
const auto &privateSyms = dsp.getAllSymbolsToPrivatize();
if (!privateSyms.empty() &&
llvm::all_of(privateSyms, [](const semantics::Symbol *sym) {
- return isNonDerivedNonAllocatableItem(*sym);
+ return isNonDerivedNonPointerItem(*sym);
}))
sectionsOp.setPrivateAssumeBoundedRecipeEffects(true);
@@ -3688,10 +3694,7 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
// The generated conditional-lastprivate recipe only accesses the
// intrinsic value and index fields of its accumulator, despite its
// record type.
- return variable == lpAlloca ||
- (!fir::isAllocatableType(variable.getType()) &&
- !fir::isa_derived(
- fir::getFortranElementType(variable.getType())));
+ return variable == lpAlloca || isNonDerivedNonPointer(variable);
}))
sectionsOp.setReductionAssumeBoundedRecipeEffects(true);
>From a12c0818be2bfe316c8931ea4d41e037286272d6 Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com" <jamie.rowlinson-patel at arm.com>
Date: Tue, 20 Oct 2026 14:40:50 +0000
Subject: [PATCH 16/17] Fix isNonDerivedNonPointer
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 6ba7da04c7d7c9..98a0a54ad2a6f2 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3573,7 +3573,8 @@ static bool isNonDerivedNonPointer(const semantics::Symbol &sym) {
static bool isNonDerivedNonPointer(const mlir::Value &variable) {
return !fir::isAllocatableType(variable.getType()) &&
- !fir::isa_derived(fir::getFortranElementType(variable.getType()));
+ !fir::isa_derived(fir::getFortranElementType(variable.getType())) &&
+ !fir::isPointerType(variable.getType());
}
static bool isNonDerivedNonPointerItem(const semantics::Symbol &sym) {
>From 68440cd1b87a419cb7d52dc1810697b907b07d32 Mon Sep 17 00:00:00 2001
From: "Jamie.Rowlinson-Patel at arm.com" <jamie.rowlinson-patel at arm.com>
Date: Fri, 30 Oct 2026 14:53:02 +0000
Subject: [PATCH 17/17] Add recursive memory effects for sections test
---
mlir/test/Dialect/OpenMP/memory-effects.mlir | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/mlir/test/Dialect/OpenMP/memory-effects.mlir b/mlir/test/Dialect/OpenMP/memory-effects.mlir
index 3110627a89b106..b52f815c5e05cb 100644
--- a/mlir/test/Dialect/OpenMP/memory-effects.mlir
+++ b/mlir/test/Dialect/OpenMP/memory-effects.mlir
@@ -20,6 +20,7 @@ func.func @master_recursive_memory_effects() {
func.func @sections_effects() {
// An implicit barrier reads and writes unspecified memory.
+ // expected-remark at below {{recursive memory effects}}
// 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 {
@@ -30,6 +31,7 @@ func.func @sections_effects() {
func.func @sections_nowait_effects() {
// Nowait removes the implicit barrier effects.
+ // expected-remark at below {{recursive memory effects}}
// expected-remark at +1 {{operation has no memory effects}}
omp.sections nowait {
omp.terminator
@@ -48,6 +50,7 @@ omp.private {type = firstprivate} @private_i32 : memref<i32> copy {
omp.private {type = private} @private_ptr : !llvm.ptr
func.func @sections_bounded_private_effects(%x: memref<i32>, %y: !llvm.ptr) {
+ // expected-remark at below {{recursive memory effects}}
// 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>'}}
@@ -65,6 +68,7 @@ func.func @sections_bounded_private_effects(%x: memref<i32>, %y: !llvm.ptr) {
func.func @sections_unbounded_private_effects(%x: memref<i32>) {
// Without the flag, all private recipe effects are arbitrary.
+ // 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 resource '<Default>'}}
@@ -92,6 +96,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>'}}
@@ -108,6 +113,7 @@ func.func @sections_bounded_reduction_effects(%x: memref<f32>) {
func.func @sections_unbounded_reduction_effects(%x: memref<f32>) {
// Without the flag, even a pure recipe is modeled with arbitrary effects.
+ // 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 resource '<Default>'}}
@@ -120,6 +126,7 @@ func.func @sections_unbounded_reduction_effects(%x: memref<f32>) {
func.func @sections_bounded_private_unbounded_reduction(%x: memref<i32>, %y: memref<f32>) {
// A bounded private clause does not constrain an unbounded reduction clause.
+ // 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 resource '<Default>'}}
@@ -133,6 +140,7 @@ func.func @sections_bounded_private_unbounded_reduction(%x: memref<i32>, %y: mem
func.func @sections_unbounded_private_bounded_reduction(%x: memref<i32>, %y: memref<f32>) {
// A bounded reduction clause does not constrain an unbounded private clause.
+ // 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 resource '<Default>'}}
@@ -146,6 +154,7 @@ 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>'}}
More information about the Mlir-commits
mailing list