[Mlir-commits] [flang] [mlir] [mlir][OpenMP] Model memory effects of omp.section and omp.sections (PR #230167)

[llvmlistbot at llvm.org]

================
@@ -3258,6 +3259,74 @@ LogicalResult SectionsOp::verifyRegions() {
   return success();
 }
 
+/// 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());
+}
+
+/// 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())) {
+    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);
+  }
+}
+
+/// Reductions initialise private accumulators and combine them into the
+/// original variables.
+static void
+getReductionEffects(SectionsOp 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);
+
+    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);
+  }
+}
+
+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);
+  getReductionEffects(*this, effects);
+  if (!getAssumeBoundedRecipeEffects() &&
+      (!getPrivateVars().empty() || !getReductionVars().empty()))
+    addUnknownMemoryEffects(effects);
----------------
JRP-arm wrote:

I've made the suggested change. However my view would be to keep it out of a helper function as reductions in different constructs are named differently. E.g. task_reduction, in_reduction so it could be neater to keep it construct-specific. 

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