[Mlir-commits] [mlir] 146d52e - [MLIR] Verify there are no side-effecting ops in GenericAtomicRMWOp body.

[Alexander Belyaev]

Author: Alexander Belyaev
Date: 2020-04-22T09:02:58+02:00
New Revision: 146d52e732877d9d74b81b7559fe6a7647351d52

URL: https://github.com/llvm/llvm-project/commit/146d52e732877d9d74b81b7559fe6a7647351d52
DIFF: https://github.com/llvm/llvm-project/commit/146d52e732877d9d74b81b7559fe6a7647351d52.diff

LOG: [MLIR] Verify there are no side-effecting ops in GenericAtomicRMWOp body.

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

Added: 
    

Modified: 
    mlir/include/mlir/Dialect/StandardOps/IR/Ops.td
    mlir/lib/Dialect/StandardOps/IR/Ops.cpp
    mlir/test/IR/invalid-ops.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Dialect/StandardOps/IR/Ops.td b/mlir/include/mlir/Dialect/StandardOps/IR/Ops.td
index 38cb8dcb3d55..e5a1af544e09 100644
--- a/mlir/include/mlir/Dialect/StandardOps/IR/Ops.td
+++ b/mlir/include/mlir/Dialect/StandardOps/IR/Ops.td
@@ -499,7 +499,8 @@ def GenericAtomicRMWOp : Std_Op<"generic_atomic_rmw", [
     The result represents the latest value that was stored. The region contains
     the code for the modification itself. The entry block has a single argument
     that represents the value stored in `memref[indices]` before the write is
-    performed.
+    performed. No side-effecting ops are allowed in the body of
+    `GenericAtomicRMWOp`.
 
     Example:
 

diff  --git a/mlir/lib/Dialect/StandardOps/IR/Ops.cpp b/mlir/lib/Dialect/StandardOps/IR/Ops.cpp
index 9ac2f4ba35e4..0a96c9a5bf44 100644
--- a/mlir/lib/Dialect/StandardOps/IR/Ops.cpp
+++ b/mlir/lib/Dialect/StandardOps/IR/Ops.cpp
@@ -507,7 +507,18 @@ static LogicalResult verify(GenericAtomicRMWOp op) {
   if (op.getResult().getType() != block.getArgument(0).getType())
     return op.emitOpError(
         "expected block argument of the same type result type");
-  return success();
+
+  bool hasSideEffects =
+      op.body()
+          .walk([&](Operation *nestedOp) {
+            if (MemoryEffectOpInterface::hasNoEffect(nestedOp))
+              return WalkResult::advance();
+            nestedOp->emitError("body of 'generic_atomic_rmw' should contain "
+                                "only operations with no side effects");
+            return WalkResult::interrupt();
+          })
+          .wasInterrupted();
+  return hasSideEffects ? failure() : success();
 }
 
 static ParseResult parseGenericAtomicRMWOp(OpAsmParser &parser,

diff  --git a/mlir/test/IR/invalid-ops.mlir b/mlir/test/IR/invalid-ops.mlir
index 17eaded116e6..fe2556d0a9d9 100644
--- a/mlir/test/IR/invalid-ops.mlir
+++ b/mlir/test/IR/invalid-ops.mlir
@@ -1179,6 +1179,18 @@ func @generic_atomic_rmw_result_type_mismatch(%I: memref<10xf32>, %i : index) {
 
 // -----
 
+func @generic_atomic_rmw_has_side_effects(%I: memref<10xf32>, %i : index) {
+  // expected-error at +4 {{should contain only operations with no side effects}}
+  %x = generic_atomic_rmw %I[%i] : memref<10xf32> {
+    ^bb0(%old_value : f32):
+      %c1 = constant 1.0 : f32
+      %buf = alloc() : memref<2048xf32>
+      atomic_yield %c1 : f32
+  }
+}
+
+// -----
+
 func @atomic_yield_type_mismatch(%I: memref<10xf32>, %i : index) {
   // expected-error at +4 {{op types mismatch between yield op: 'i32' and its parent: 'f32'}}
   %x = generic_atomic_rmw %I[%i] : memref<10xf32> {