[flang-commits] [flang] 7ddb674 - [mlir][acc] Declare the memory effects of acc.atomic.read (#228899)
via flang-commits
flang-commits at lists.llvm.org
Wed Oct 7 22:46:10 PDT 2026
Author: Kareem Ergawy
Date: 2026-10-08T07:46:04+02:00
New Revision: 7ddb67460b168b17ebb263dc99bf956b56d7e517
URL: https://github.com/llvm/llvm-project/commit/7ddb67460b168b17ebb263dc99bf956b56d7e517
DIFF: https://github.com/llvm/llvm-project/commit/7ddb67460b168b17ebb263dc99bf956b56d7e517.diff
LOG: [mlir][acc] Declare the memory effects of acc.atomic.read (#228899)
The operation read `x` and wrote `v` but declared no memory effects at
all, so every memory analysis had to treat both locations as unknown and
stay maximally conservative. In particular it could not be seen to
overwrite `v`.
Declare the effects per operand so the read on `x` and the write on `v`
are visible.
Added:
flang/test/Transforms/licm-acc-atomic.fir
mlir/test/Dialect/OpenACC/side-effects.mlir
Modified:
mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
Removed:
################################################################################
diff --git a/flang/test/Transforms/licm-acc-atomic.fir b/flang/test/Transforms/licm-acc-atomic.fir
new file mode 100644
index 0000000000000..041c6c50995fb
--- /dev/null
+++ b/flang/test/Transforms/licm-acc-atomic.fir
@@ -0,0 +1,95 @@
+// Test what the declared memory effects of acc.atomic.read permit around it.
+// RUN: fir-opt %s --flang-licm | FileCheck %s
+// RUN: fir-opt %s --loop-invariant-code-motion | FileCheck %s --check-prefix=GENERIC
+// RUN: fir-opt %s -cse | FileCheck %s --check-prefix=CSE
+
+// acc.atomic.read declares its effects per operand: it reads the location
+// designated by `x` and writes the location designated by `v`. It declares
+// nothing else, so it is modelled with relaxed ordering: the access to the
+// location is atomic, but no ordering relationship is created with accesses to
+// any other location, and the optimizer may move those across it.
+//
+// Without the declared effects the operation implements no
+// MemoryEffectOpInterface at all. fir::AliasAnalysis::getModRef then falls into
+// its `if (!interface) return getModAndRef()` branch, so the operation reads as
+// "may modify everything" and no load in the enclosing loop can be hoisted,
+// however unrelated it is.
+
+// A load of a location the atomic neither reads nor writes is free to leave the
+// loop. This is the case the declared effects unlock.
+
+// CHECK-LABEL: func.func @hoist_unrelated_load_across_atomic
+// CHECK: fir.load
+// CHECK: scf.for
+// CHECK-NOT: fir.load
+// CHECK: acc.atomic.read
+func.func @hoist_unrelated_load_across_atomic(
+ %x: !fir.ref<i32>, %v: !fir.ref<i32>, %y: !fir.ref<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.read %v = %x : !fir.ref<i32>, !fir.ref<i32>, i32
+ }
+ return
+}
+
+// The effects are precise per operand rather than a blanket "touches memory":
+// `v` is written, so a load of `v` is not loop invariant and stays put.
+
+// CHECK-LABEL: func.func @no_hoist_load_of_atomic_destination
+// CHECK: scf.for
+// CHECK: fir.load
+// CHECK: acc.atomic.read
+func.func @no_hoist_load_of_atomic_destination(
+ %x: !fir.ref<i32>, %v: !fir.ref<i32>, %n: index) {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ scf.for %i = %c0 to %n step %c1 {
+ %0 = fir.load %v : !fir.ref<i32>
+ acc.atomic.read %v = %x : !fir.ref<i32>, !fir.ref<i32>, i32
+ }
+ return
+}
+
+// `x` is only read, so a load of `x` does not conflict with the atomic and is
+// hoisted. Together with the previous function this pins that the two operands
+// carry
diff erent effects.
+
+// CHECK-LABEL: func.func @hoist_load_of_atomic_source
+// CHECK: fir.load
+// CHECK: scf.for
+// CHECK-NOT: fir.load
+// CHECK: acc.atomic.read
+func.func @hoist_load_of_atomic_source(
+ %x: !fir.ref<i32>, %v: !fir.ref<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.read %v = %x : !fir.ref<i32>, !fir.ref<i32>, i32
+ }
+ return
+}
+
+// The generic MLIR loop-invariant-code-motion pass hoists on
+// isMemoryEffectFree/isSpeculatable and consults no alias analysis, so it moves
+// nothing here with or without the declared effects. Recorded so the contrast
+// with flang-licm above is explicit.
+
+// GENERIC-LABEL: func.func @hoist_unrelated_load_across_atomic
+// GENERIC: scf.for
+// GENERIC: fir.load
+// GENERIC: acc.atomic.read
+
+// An atomic read is not removed by common subexpression elimination - it has
+// declared effects, so two reads of the same location both survive.
+
+// CSE-LABEL: func.func @cse_keeps_both_atomic_reads
+// CSE: acc.atomic.read
+// CSE: acc.atomic.read
+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
+ acc.atomic.read %v = %x : !fir.ref<i32>, !fir.ref<i32>, i32
+ return
+}
diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index b7914a867c8d2..a0e1aa0417687 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -3042,10 +3042,20 @@ def AtomicReadOp : OpenACC_Op<"atomic.read", [AtomicReadOpInterface]> {
The operand `x` is the address from where the value is atomically read.
The operand `v` is the address where the value is stored after reading.
+
+ The operation declares its memory effects per operand: it reads the
+ location designated by `x` and writes the location designated by `v`. It
+ declares no other effect, 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 - unlike OpenMP
+ it has no memory-ordering clause - so relaxed is the weakest reading
+ consistent with the specification and the one that permits the most
+ optimization.
}];
- let arguments = (ins OpenACC_PointerLikeType:$x,
- OpenACC_PointerLikeType:$v,
+ let arguments = (ins Arg<OpenACC_PointerLikeType, "address atomically read from", [MemRead]>:$x,
+ Arg<OpenACC_PointerLikeType, "address storing read value", [MemWrite]>:$v,
TypeAttr:$element_type, Optional<I1>:$ifCond);
let assemblyFormat = [{
oilist(
diff --git a/mlir/test/Dialect/OpenACC/side-effects.mlir b/mlir/test/Dialect/OpenACC/side-effects.mlir
new file mode 100644
index 0000000000000..1a7f98cefba8c
--- /dev/null
+++ b/mlir/test/Dialect/OpenACC/side-effects.mlir
@@ -0,0 +1,42 @@
+// RUN: mlir-opt %s --test-side-effects --verify-diagnostics --split-input-file
+
+// acc.atomic.read declares its effects per operand: it reads the location
+// designated by `x` and writes the location designated by `v`. Without them the
+// operation reports no effects at all, which every memory analysis has to read
+// as "unknown" and treat maximally conservatively -- in particular it cannot be
+// seen to overwrite `v`.
+
+func.func @atomic_read(%x: memref<i32>, %v: memref<i32>) {
+ // expected-remark @below {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
+ // expected-remark @below {{found an instance of 'write' on op operand 1, on resource '<Default>'}}
+ acc.atomic.read %v = %x : memref<i32>, memref<i32>, i32
+ return
+}
+
+// -----
+
+// The same effects are reported inside an acc.atomic.capture region. The
+// capture itself carries RecursiveMemoryEffects and so does not implement
+// MemoryEffectOpInterface; the test pass walks only operations that do, so the
+// capture gets no remark of its own and the effects come from its body. The
+// read writes `v` and reads `x`; the update both reads and writes `x`, which is
+// what keeps `x` from being treated as overwritten.
+
+func.func @atomic_capture(%x: memref<i32>, %v: memref<i32>) {
+ acc.atomic.capture {
+ // expected-remark @below {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
+ // expected-remark @below {{found an instance of 'write' on op operand 1, on resource '<Default>'}}
+ acc.atomic.read %v = %x : memref<i32>, memref<i32>, i32
+ // expected-remark @below {{found an instance of 'read' on op operand 0, on resource '<Default>'}}
+ // expected-remark @below {{found an instance of 'write' on op operand 0, on resource '<Default>'}}
+ acc.atomic.update %x : memref<i32> {
+ ^bb0(%arg0: i32):
+ // expected-remark @below {{operation has no memory effects}}
+ %0 = arith.constant 1 : i32
+ // expected-remark @below {{operation has no memory effects}}
+ %1 = arith.addi %arg0, %0 : i32
+ acc.yield %1 : i32
+ }
+ }
+ return
+}
More information about the flang-commits
mailing list