[Mlir-commits] [flang] [mlir] [mlir][acc] Declare the memory effects of acc.atomic.write (PR #230029)
Kareem Ergawy
llvmlistbot at llvm.org
Fri Oct 9 00:41:55 PDT 2026
https://github.com/ergawy updated https://github.com/llvm/llvm-project/pull/230029
>From 639d9f462f6cc0db27901c18a256ab360490b567 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at gmail.com>
Date: Thu, 8 Oct 2026 01:23:30 -0700
Subject: [PATCH 1/2] [mlir][acc] Declare the memory effects of
acc.atomic.write
The operation wrote the location designated by `x` but declared no memory
effects, so it did not implement MemoryEffectOpInterface at all and every
memory analysis had to treat it as unknown. fir::AliasAnalysis::getModRef
returns ModAndRef for such an operation, so a loop containing one blocked
the hoisting of any load, however unrelated.
Declare the write on `x`. `expr` is a value rather than a location and
carries no effect.
This completes the set: acc.atomic.update already declared MemRead and
MemWrite on `x`, acc.atomic.read declares its effects since #228899, and
acc.atomic.capture derives its effects from its body.
The relaxed-ordering contract is stated in the operation description and
tested the same way as for acc.atomic.read: an unrelated load is hoisted
out of a loop across the write, and a load of `x` is not.
---
flang/test/Transforms/licm-acc-atomic.fir | 35 +++++++++++++++++++
.../mlir/Dialect/OpenACC/OpenACCOps.td | 12 ++++++-
mlir/test/Dialect/OpenACC/side-effects.mlir | 12 +++++++
3 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/flang/test/Transforms/licm-acc-atomic.fir b/flang/test/Transforms/licm-acc-atomic.fir
index 041c6c50995fb3..afbca802ebfd42 100644
--- a/flang/test/Transforms/licm-acc-atomic.fir
+++ b/flang/test/Transforms/licm-acc-atomic.fir
@@ -93,3 +93,38 @@ func.func @cse_keeps_both_atomic_reads(%x: !fir.ref<i32>, %v: !fir.ref<i32>) {
acc.atomic.read %v = %x : !fir.ref<i32>, !fir.ref<i32>, i32
return
}
+
+// The same contract for acc.atomic.write: it names only `x`, so an unrelated
+// load is free to leave the loop, and a load of `x` is not.
+
+// CHECK-LABEL: func.func @write_hoist_unrelated_load
+// CHECK: fir.load
+// CHECK: scf.for
+// CHECK-NOT: fir.load
+// CHECK: acc.atomic.write
+func.func @write_hoist_unrelated_load(
+ %x: !fir.ref<i32>, %y: !fir.ref<i32>, %val: i32, %n: index) {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ scf.for %i = %c0 to %n step %c1 {
+ %0 = fir.load %y : !fir.ref<i32>
+ acc.atomic.write %x = %val : !fir.ref<i32>, i32
+ }
+ return
+}
+
+// CHECK-LABEL: func.func @write_no_hoist_load_of_x
+// CHECK: scf.for
+// CHECK: fir.load
+// CHECK: acc.atomic.write
+func.func @write_no_hoist_load_of_x(
+ %x: !fir.ref<i32>, %val: i32, %n: index) {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ scf.for %i = %c0 to %n step %c1 {
+ %0 = fir.load %x : !fir.ref<i32>
+ acc.atomic.write %x = %val : !fir.ref<i32>, i32
+ }
+ return
+}
+
diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index 2e8cfc5f68ef2e..70f8ee5c381721 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -3079,9 +3079,19 @@ def AtomicWriteOp : OpenACC_Op<"atomic.write",[AtomicWriteOpInterface]> {
written w.r.t. multiple threads. The evaluation of `expr` need not be
atomic w.r.t. the write to address. In general, the type(x) must
dereference to type(expr).
+
+ The operation declares its memory effects per operand: it writes the
+ location designated by `x`. `expr` is a value rather than a location, so
+ it carries no effect. No other effect is declared, which models relaxed
+ ordering: the access to the location is atomic, but the operation creates
+ no ordering relationship with accesses to any other location, so those may
+ be moved across it. OpenACC leaves the ordering of the `atomic` construct
+ unspecified, so relaxed is the weakest reading consistent with the
+ specification and the one that permits the most optimization.
}];
- let arguments = (ins OpenACC_PointerLikeType:$x,
+ let arguments = (ins Arg<OpenACC_PointerLikeType,
+ "address atomically written to", [MemWrite]>:$x,
AnyType:$expr,
Optional<I1>:$ifCond);
let assemblyFormat = [{
diff --git a/mlir/test/Dialect/OpenACC/side-effects.mlir b/mlir/test/Dialect/OpenACC/side-effects.mlir
index 1a7f98cefba8c3..59b138c72ee539 100644
--- a/mlir/test/Dialect/OpenACC/side-effects.mlir
+++ b/mlir/test/Dialect/OpenACC/side-effects.mlir
@@ -40,3 +40,15 @@ func.func @atomic_capture(%x: memref<i32>, %v: memref<i32>) {
}
return
}
+
+// -----
+
+// acc.atomic.write declares a write on the location designated by `x`. `expr`
+// is a value rather than a location, so it carries no effect.
+
+func.func @atomic_write(%x: memref<i32>, %v: i32) {
+ // expected-remark @below {{found an instance of 'write' on op operand 0, on resource '<Default>'}}
+ acc.atomic.write %x = %v : memref<i32>, i32
+ return
+}
+
>From 5f8e6458c7fb78bcf921851a579345125aaa8cc4 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at gmail.com>
Date: Thu, 8 Oct 2026 22:33:56 -0700
Subject: [PATCH 2/2] [flang][acc] Test LICM around acc.atomic.capture
acc.atomic.capture declares no memory effects of its own - it carries
RecursiveMemoryEffects - so fir::AliasAnalysis::getModRef derives them by
recursing into its region. That recursion is only precise once every member
declares its own effects: a member without MemoryEffectOpInterface makes it
fall back to ModRef over the whole region, and nothing in the enclosing loop
can be hoisted.
Cover both forms the interface allows. For read+update, an unrelated load is
hoisted out of the loop while loads of `v` and `x` are not, `v` being written
by the read and `x` read and written by the update. For read+write, the same
unrelated load is hoisted and a load of `x` is not.
---
flang/test/Transforms/licm-acc-atomic.fir | 123 ++++++++++++++++++++++
1 file changed, 123 insertions(+)
diff --git a/flang/test/Transforms/licm-acc-atomic.fir b/flang/test/Transforms/licm-acc-atomic.fir
index afbca802ebfd42..fd52d4991c1e65 100644
--- a/flang/test/Transforms/licm-acc-atomic.fir
+++ b/flang/test/Transforms/licm-acc-atomic.fir
@@ -128,3 +128,126 @@ func.func @write_no_hoist_load_of_x(
return
}
+//--- acc.atomic.capture ------------------------------------------------------
+
+// acc.atomic.capture carries RecursiveMemoryEffects and declares no effects of
+// its own, so fir::AliasAnalysis::getModRef derives them by recursing into the
+// region. That is only precise once every member declares its own effects: an
+// operation without MemoryEffectOpInterface makes the recursion fall back to
+// ModRef, which is the whole region, and nothing in the loop can be hoisted.
+
+// Capture of the read+update form. The unrelated load leaves the loop; `v` and
+// `x` are both written, so loads of those stay.
+
+// CHECK-LABEL: func.func @capture_hoist_unrelated_load
+// CHECK: fir.load
+// CHECK: scf.for
+// CHECK-NOT: fir.load
+// CHECK: acc.atomic.capture
+func.func @capture_hoist_unrelated_load(
+ %x: !fir.ref<i32>, %v: !fir.ref<i32>, %y: !fir.ref<i32>, %n: index) {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ %one = arith.constant 1 : i32
+ scf.for %i = %c0 to %n step %c1 {
+ %0 = fir.load %y : !fir.ref<i32>
+ acc.atomic.capture {
+ acc.atomic.read %v = %x : !fir.ref<i32>, !fir.ref<i32>, i32
+ acc.atomic.update %x : !fir.ref<i32> {
+ ^bb0(%old: i32):
+ %r = arith.addi %old, %one : i32
+ acc.yield %r : i32
+ }
+ }
+ }
+ return
+}
+
+// CHECK-LABEL: func.func @capture_no_hoist_load_of_v
+// CHECK: scf.for
+// CHECK: fir.load
+// CHECK: acc.atomic.capture
+func.func @capture_no_hoist_load_of_v(
+ %x: !fir.ref<i32>, %v: !fir.ref<i32>, %n: index) {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ %one = arith.constant 1 : i32
+ scf.for %i = %c0 to %n step %c1 {
+ %0 = fir.load %v : !fir.ref<i32>
+ acc.atomic.capture {
+ acc.atomic.read %v = %x : !fir.ref<i32>, !fir.ref<i32>, i32
+ acc.atomic.update %x : !fir.ref<i32> {
+ ^bb0(%old: i32):
+ %r = arith.addi %old, %one : i32
+ acc.yield %r : i32
+ }
+ }
+ }
+ return
+}
+
+// CHECK-LABEL: func.func @capture_no_hoist_load_of_x
+// CHECK: scf.for
+// CHECK: fir.load
+// CHECK: acc.atomic.capture
+func.func @capture_no_hoist_load_of_x(
+ %x: !fir.ref<i32>, %v: !fir.ref<i32>, %n: index) {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ %one = arith.constant 1 : i32
+ scf.for %i = %c0 to %n step %c1 {
+ %0 = fir.load %x : !fir.ref<i32>
+ acc.atomic.capture {
+ acc.atomic.read %v = %x : !fir.ref<i32>, !fir.ref<i32>, i32
+ acc.atomic.update %x : !fir.ref<i32> {
+ ^bb0(%old: i32):
+ %r = arith.addi %old, %one : i32
+ acc.yield %r : i32
+ }
+ }
+ }
+ return
+}
+
+// Capture of the read+write form. This is the one the write effect above
+// unlocks: with the write opaque the whole capture read as ModRef and the
+// unrelated load stayed in the loop.
+
+// CHECK-LABEL: func.func @capture_write_hoist_unrelated_load
+// CHECK: fir.load
+// CHECK: scf.for
+// CHECK-NOT: fir.load
+// CHECK: acc.atomic.capture
+func.func @capture_write_hoist_unrelated_load(
+ %x: !fir.ref<i32>, %v: !fir.ref<i32>, %y: !fir.ref<i32>, %val: i32,
+ %n: index) {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ scf.for %i = %c0 to %n step %c1 {
+ %0 = fir.load %y : !fir.ref<i32>
+ acc.atomic.capture {
+ acc.atomic.read %v = %x : !fir.ref<i32>, !fir.ref<i32>, i32
+ acc.atomic.write %x = %val : !fir.ref<i32>, i32
+ }
+ }
+ return
+}
+
+// CHECK-LABEL: func.func @capture_write_no_hoist_load_of_x
+// CHECK: scf.for
+// CHECK: fir.load
+// CHECK: acc.atomic.capture
+func.func @capture_write_no_hoist_load_of_x(
+ %x: !fir.ref<i32>, %v: !fir.ref<i32>, %val: i32, %n: index) {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ scf.for %i = %c0 to %n step %c1 {
+ %0 = fir.load %x : !fir.ref<i32>
+ acc.atomic.capture {
+ acc.atomic.read %v = %x : !fir.ref<i32>, !fir.ref<i32>, i32
+ acc.atomic.write %x = %val : !fir.ref<i32>, i32
+ }
+ }
+ return
+}
+
More information about the Mlir-commits
mailing list