[Mlir-commits] [mlir] 8c1553f - [mlir][spirv] Add memory semantics verify for atomic operations

[llvmlistbot at llvm.org]

Author: xndcn
Date: 2021-10-14T00:00:55+08:00
New Revision: 8c1553f0d7bd1d78a9e7e742099372ad5bf647aa

URL: https://github.com/llvm/llvm-project/commit/8c1553f0d7bd1d78a9e7e742099372ad5bf647aa
DIFF: https://github.com/llvm/llvm-project/commit/8c1553f0d7bd1d78a9e7e742099372ad5bf647aa.diff

LOG: [mlir][spirv] Add memory semantics verify for atomic operations

Differential Revision: https://reviews.llvm.org/D111510

Added: 
    

Modified: 
    mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBarrierOps.td
    mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
    mlir/test/Dialect/SPIRV/IR/atomic-ops.mlir
    mlir/utils/spirv/report_coverage.sh

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBarrierOps.td b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBarrierOps.td
index 98a0c300e69f0..79e4ef55014cb 100644
--- a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBarrierOps.td
+++ b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBarrierOps.td
@@ -77,7 +77,7 @@ def SPV_ControlBarrierOp : SPV_Op<"ControlBarrier", []> {
 
   let results = (outs);
 
-  let verifier = [{ return verifyMemorySemantics(*this); }];
+  let verifier = [{ return verifyMemorySemantics(getOperation(), memory_semantics()); }];
 
   let autogenSerialization = 0;
 
@@ -131,7 +131,7 @@ def SPV_MemoryBarrierOp : SPV_Op<"MemoryBarrier", []> {
 
   let results = (outs);
 
-  let verifier = [{ return verifyMemorySemantics(*this); }];
+  let verifier = [{ return verifyMemorySemantics(getOperation(), memory_semantics()); }];
 
   let autogenSerialization = 0;
 

diff  --git a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
index 9a2662211c7c6..14d58ef107684 100644
--- a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
+++ b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
@@ -497,15 +497,14 @@ static LogicalResult verifySourceMemoryAccessAttribute(MemoryOpTy memoryOp) {
   return success();
 }
 
-template <typename BarrierOp>
-static LogicalResult verifyMemorySemantics(BarrierOp op) {
+static LogicalResult
+verifyMemorySemantics(Operation *op, spirv::MemorySemantics memorySemantics) {
   // According to the SPIR-V specification:
   // "Despite being a mask and allowing multiple bits to be combined, it is
   // invalid for more than one of these four bits to be set: Acquire, Release,
   // AcquireRelease, or SequentiallyConsistent. Requesting both Acquire and
   // Release semantics is done by setting the AcquireRelease bit, not by setting
   // two bits."
-  auto memorySemantics = op.memory_semantics();
   auto atMostOneInSet = spirv::MemorySemantics::Acquire |
                         spirv::MemorySemantics::Release |
                         spirv::MemorySemantics::AcquireRelease |
@@ -514,9 +513,10 @@ static LogicalResult verifyMemorySemantics(BarrierOp op) {
   auto bitCount = llvm::countPopulation(
       static_cast<uint32_t>(memorySemantics & atMostOneInSet));
   if (bitCount > 1) {
-    return op.emitError("expected at most one of these four memory constraints "
-                        "to be set: `Acquire`, `Release`,"
-                        "`AcquireRelease` or `SequentiallyConsistent`");
+    return op->emitError(
+        "expected at most one of these four memory constraints "
+        "to be set: `Acquire`, `Release`,"
+        "`AcquireRelease` or `SequentiallyConsistent`");
   }
   return success();
 }
@@ -772,6 +772,11 @@ static LogicalResult verifyAtomicUpdateOp(Operation *op) {
                              "pointer operand's pointee type ")
              << elementType << ", but found " << valueType;
   }
+  auto memorySemantics = static_cast<spirv::MemorySemantics>(
+      op->getAttrOfType<IntegerAttr>(kSemanticsAttrName).getInt());
+  if (failed(verifyMemorySemantics(op, memorySemantics))) {
+    return failure();
+  }
   return success();
 }
 

diff  --git a/mlir/test/Dialect/SPIRV/IR/atomic-ops.mlir b/mlir/test/Dialect/SPIRV/IR/atomic-ops.mlir
index 67529f289bd5b..7a10878fad5ef 100644
--- a/mlir/test/Dialect/SPIRV/IR/atomic-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/atomic-ops.mlir
@@ -29,6 +29,14 @@ func @atomic_and(%ptr : !spv.ptr<i32, StorageBuffer>, %value : i64) -> i64 {
 
 // -----
 
+func @atomic_and(%ptr : !spv.ptr<i32, StorageBuffer>, %value : i32) -> i32 {
+  // expected-error @+1 {{expected at most one of these four memory constraints to be set: `Acquire`, `Release`,`AcquireRelease` or `SequentiallyConsistent`}}
+  %0 = spv.AtomicAnd "Device" "Acquire|Release" %ptr, %value : !spv.ptr<i32, StorageBuffer>
+  return %0 : i32
+}
+
+// -----
+
 //===----------------------------------------------------------------------===//
 // spv.AtomicCompareExchangeWeak
 //===----------------------------------------------------------------------===//

diff  --git a/mlir/utils/spirv/report_coverage.sh b/mlir/utils/spirv/report_coverage.sh
index 63f2f3db0cf99..7ef1cf2a2140f 100755
--- a/mlir/utils/spirv/report_coverage.sh
+++ b/mlir/utils/spirv/report_coverage.sh
@@ -16,5 +16,5 @@ current_file="$(readlink -f "$0")"
 current_dir="$(dirname "$current_file")"
 
 python3 ${current_dir}/gen_spirv_dialect.py \
-  --base-td-path ${current_dir}/../../include/mlir/Dialect/SPIRV/SPIRVBase.td \
+  --base-td-path ${current_dir}/../../include/mlir/Dialect/SPIRV/IR/SPIRVBase.td \
   --gen-inst-coverage