[Mlir-commits] [mlir] 1bc1609 - [mlir][acc] Declare the memory effects of acc.atomic.write (#230029)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Fri Oct 9 01:38:30 PDT 2026
Author: Kareem Ergawy
Date: 2026-10-09T10:38:24+02:00
New Revision: 1bc1609a565c65a5f28c2c49a5ebe9c74e13c1d8
URL: https://github.com/llvm/llvm-project/commit/1bc1609a565c65a5f28c2c49a5ebe9c74e13c1d8
DIFF: https://github.com/llvm/llvm-project/commit/1bc1609a565c65a5f28c2c49a5ebe9c74e13c1d8.diff
LOG: [mlir][acc] Declare the memory effects of acc.atomic.write (#230029)
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.
Added:
Modified:
flang/test/Transforms/licm-acc-atomic.fir
mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
mlir/test/Dialect/OpenACC/side-effects.mlir
Removed:
################################################################################
diff --git a/flang/test/Transforms/licm-acc-atomic.fir b/flang/test/Transforms/licm-acc-atomic.fir
index 041c6c50995fb3..fd52d4991c1e65 100644
--- a/flang/test/Transforms/licm-acc-atomic.fir
+++ b/flang/test/Transforms/licm-acc-atomic.fir
@@ -93,3 +93,161 @@ 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
+}
+
+//--- 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
+}
+
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
+}
+
More information about the Mlir-commits
mailing list