From 146d52e732877d9d74b81b7559fe6a7647351d52 Mon Sep 17 00:00:00 2001 From: Alexander Belyaev Date: Wed, 22 Apr 2020 09:02:00 +0200 Subject: [PATCH] [MLIR] Verify there are no side-effecting ops in GenericAtomicRMWOp body. Differential Revision: https://reviews.llvm.org/D78559 --- mlir/include/mlir/Dialect/StandardOps/IR/Ops.td | 3 ++- mlir/lib/Dialect/StandardOps/IR/Ops.cpp | 13 ++++++++++++- mlir/test/IR/invalid-ops.mlir | 12 ++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/mlir/include/mlir/Dialect/StandardOps/IR/Ops.td b/mlir/include/mlir/Dialect/StandardOps/IR/Ops.td index 38cb8dc..e5a1af5 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 9ac2f4b..0a96c9a 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 17eaded..fe2556d 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@+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@+4 {{op types mismatch between yield op: 'i32' and its parent: 'f32'}} %x = generic_atomic_rmw %I[%i] : memref<10xf32> { -- 2.7.4