[Mlir-commits] [mlir] 1854a88 - [mlir][SPIR-V] Fix CopyMemory handling of a lone memory operands mask (#226969)

[llvmlistbot at llvm.org]

Author: Arseniy Obolenskiy
Date: 2026-10-01T06:27:06Z
New Revision: 1854a889a3239e9d7391d65fc7c7dfae684fe63d

URL: https://github.com/llvm/llvm-project/commit/1854a889a3239e9d7391d65fc7c7dfae684fe63d
DIFF: https://github.com/llvm/llvm-project/commit/1854a889a3239e9d7391d65fc7c7dfae684fe63d.diff

LOG: [mlir][SPIR-V] Fix CopyMemory handling of a lone memory operands mask (#226969)

Fix `spirv.CopyMemory` with a lone memory operands mask, which per spec
applies to both operands

Verifier no longer rejects MakePointerVisible on it

Added: 
    

Modified: 
    mlir/lib/Dialect/SPIRV/IR/MemoryOps.cpp
    mlir/test/Dialect/SPIRV/IR/memory-ops.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Dialect/SPIRV/IR/MemoryOps.cpp b/mlir/lib/Dialect/SPIRV/IR/MemoryOps.cpp
index 0b98825774c80..debce94cd98cd 100644
--- a/mlir/lib/Dialect/SPIRV/IR/MemoryOps.cpp
+++ b/mlir/lib/Dialect/SPIRV/IR/MemoryOps.cpp
@@ -109,19 +109,19 @@ static void printSourceMemoryAccessAttribute(
     std::optional<spirv::MemoryAccess> memoryAccessAtrrValue = std::nullopt,
     std::optional<uint32_t> alignmentAttrValue = std::nullopt) {
 
-  printer << ", ";
-
   // Print optional memory access attribute.
-  if (auto memAccess = (memoryAccessAtrrValue ? memoryAccessAtrrValue
-                                              : memoryOp.getMemoryAccess())) {
+  if (auto memAccess =
+          (memoryAccessAtrrValue ? memoryAccessAtrrValue
+                                 : memoryOp.getSourceMemoryAccess())) {
     elidedAttrs.push_back(memoryOp.getSourceMemoryAccessAttrName());
 
-    printer << " [\"" << stringifyMemoryAccess(*memAccess) << "\"";
+    printer << ", [\"" << stringifyMemoryAccess(*memAccess) << "\"";
 
     if (spirv::bitEnumContainsAll(*memAccess, spirv::MemoryAccess::Aligned)) {
       // Print integer alignment attribute.
-      if (auto alignment = (alignmentAttrValue ? alignmentAttrValue
-                                               : memoryOp.getAlignment())) {
+      if (auto alignment =
+              (alignmentAttrValue ? alignmentAttrValue
+                                  : memoryOp.getSourceAlignment())) {
         elidedAttrs.push_back(memoryOp.getSourceAlignmentAttrName());
         printer << ", " << *alignment;
       }
@@ -208,10 +208,16 @@ static LogicalResult verifyMemoryAccessAttribute(MemoryOpTy memoryOp) {
 
   // MakePointerVisible applies to reads through the pointer, so it is invalid
   // for Store and for the Target operand of CopyMemory, both of which only
-  // write through it.
+  // write through it. A CopyMemory mask is Target-only when a Source mask
+  // follows it, otherwise it applies to both operands.
   if constexpr (std::is_same_v<MemoryOpTy, StoreOp> ||
                 std::is_same_v<MemoryOpTy, CopyMemoryOp>) {
-    if (spirv::bitEnumContainsAll(memAccess.getValue(),
+    bool sourceOnly = true;
+    if constexpr (std::is_same_v<MemoryOpTy, CopyMemoryOp>)
+      sourceOnly = memoryOp.getSourceMemoryAccess().has_value();
+
+    if (sourceOnly &&
+        spirv::bitEnumContainsAll(memAccess.getValue(),
                                   spirv::MemoryAccess::MakePointerVisible)) {
       return memoryOp.emitOpError(
           "not compatible with memory operand 'MakePointerVisible'");

diff  --git a/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir b/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir
index 78dabf67ece35..2f823198b804a 100644
--- a/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir
@@ -746,7 +746,7 @@ func.func @copy_memory_target_bad_operand() {
   %0 = spirv.Variable : !spirv.ptr<f32, Function>
   %1 = spirv.Variable : !spirv.ptr<f32, Function>
   // expected-error @+1 {{op not compatible with memory operand 'MakePointerVisible'}}
-  "spirv.CopyMemory"(%0, %1) {memory_access=#spirv.memory_access<MakePointerVisible|NonPrivatePointer>} : (!spirv.ptr<f32, Function>, !spirv.ptr<f32, Function>) -> ()
+  "spirv.CopyMemory"(%0, %1) {memory_access=#spirv.memory_access<MakePointerVisible|NonPrivatePointer>, source_memory_access=#spirv.memory_access<None>} : (!spirv.ptr<f32, Function>, !spirv.ptr<f32, Function>) -> ()
   spirv.Return
 }
 
@@ -788,6 +788,17 @@ func.func @copy_memory_print_maa() {
   // CHECK: spirv.CopyMemory "Function" %{{.*}}, "Function" %{{.*}} ["Aligned", 4], ["Aligned", 8] : f32
   "spirv.CopyMemory"(%0, %1) {source_memory_access=#spirv.memory_access<Aligned>, memory_access=#spirv.memory_access<Aligned>, source_alignment=8 : i32, alignment=4 : i32} : (!spirv.ptr<f32, Function>, !spirv.ptr<f32, Function>) -> ()
 
+  // A lone mask applies to both operands, so it may carry MakePointerVisible,
+  // MakePointerAvailable, or both. It must stay a lone mask when printed.
+  // CHECK: spirv.CopyMemory "Function" %{{.*}}, "Function" %{{.*}} ["MakePointerVisible|NonPrivatePointer"] : f32
+  "spirv.CopyMemory"(%0, %1) {memory_access=#spirv.memory_access<MakePointerVisible|NonPrivatePointer>} : (!spirv.ptr<f32, Function>, !spirv.ptr<f32, Function>) -> ()
+
+  // CHECK: spirv.CopyMemory "Function" %{{.*}}, "Function" %{{.*}} ["MakePointerAvailable|MakePointerVisible|NonPrivatePointer"] : f32
+  "spirv.CopyMemory"(%0, %1) {memory_access=#spirv.memory_access<MakePointerAvailable|MakePointerVisible|NonPrivatePointer>} : (!spirv.ptr<f32, Function>, !spirv.ptr<f32, Function>) -> ()
+
+  // CHECK: spirv.CopyMemory "Function" %{{.*}}, "Function" %{{.*}} ["MakePointerAvailable|NonPrivatePointer"], ["MakePointerVisible|NonPrivatePointer"] : f32
+  "spirv.CopyMemory"(%0, %1) {memory_access=#spirv.memory_access<MakePointerAvailable|NonPrivatePointer>, source_memory_access=#spirv.memory_access<MakePointerVisible|NonPrivatePointer>} : (!spirv.ptr<f32, Function>, !spirv.ptr<f32, Function>) -> ()
+
   spirv.Return
 }