[Mlir-commits] [mlir] [mlir][acc] Declare the memory effects of acc.atomic.read (PR #228899)

[Kareem Ergawy]

https://github.com/ergawy created https://github.com/llvm/llvm-project/pull/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.

>From 40534a23031553444833835b52219bc49d01dc7b Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at gmail.com>
Date: Sun, 4 Oct 2026 07:51:34 -0700
Subject: [PATCH] [mlir][acc] Declare the memory effects of acc.atomic.read

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.
---
 .../mlir/Dialect/OpenACC/OpenACCOps.td        |  4 +-
 mlir/test/Dialect/OpenACC/side-effects.mlir   | 42 +++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)
 create mode 100644 mlir/test/Dialect/OpenACC/side-effects.mlir

diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index b7914a867c8d2..f405237a067b2 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -3044,8 +3044,8 @@ def AtomicReadOp : OpenACC_Op<"atomic.read", [AtomicReadOpInterface]> {
     The operand `v` is the address where the value is stored after reading.
   }];
 
-  let arguments = (ins OpenACC_PointerLikeType:$x,
-                       OpenACC_PointerLikeType:$v,
+  let arguments = (ins Arg<OpenACC_PointerLikeType, "", [MemRead]>:$x,
+                       Arg<OpenACC_PointerLikeType, "", [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
+}