diff options
author | Chia-hung Duan <chiahungduan@google.com> | 2022-03-11 01:10:45 +0300 |
---|---|---|
committer | Chia-hung Duan <chiahungduan@google.com> | 2022-03-11 04:16:28 +0300 |
commit | ed645f63362e439dfed71e60595e86b906fea810 (patch) | |
tree | 7568e3485a66f5b505d721f52da7d0e80213f9b5 /mlir | |
parent | fc968bcba4d7e02390f1c1cba76a6a2cad1ae59f (diff) |
[mlir] Support verification order (3/3)
In this CL, update the function name of verifier according to the
behavior. If a verifier needs to access the region then it'll be updated
to `verifyRegions`.
Reviewed By: rriddle
Differential Revision: https://reviews.llvm.org/D120373
Diffstat (limited to 'mlir')
35 files changed, 173 insertions, 113 deletions
diff --git a/mlir/docs/OpDefinitions.md b/mlir/docs/OpDefinitions.md index dc25ef0de5f7..5d90a4bdfec6 100644 --- a/mlir/docs/OpDefinitions.md +++ b/mlir/docs/OpDefinitions.md @@ -565,18 +565,16 @@ _additional_ verification, you can use ```tablegen let hasVerifier = 1; -``` - -or - -```tablegen let hasRegionVerifier = 1; ``` -This will generate either `LogicalResult verify()` or -`LogicalResult verifyRegions()` method declaration on the op class -that can be defined with any additional verification constraints. These method -will be invoked on its verification order. +This will generate `LogicalResult verify()`/`LogicalResult verifyRegions()` +method declarations on the op class that can be defined with any additional +verification constraints. For verificaiton which needs to access the nested +operations, you should use `hasRegionVerifier` to ensure that it won't access +any ill-formed operation. Except that, The other verifications can be +implemented with `hasVerifier`. Check the next section for the execution order +of these verification methods. #### Verification Ordering diff --git a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td index 50661565add5..f13014396db4 100644 --- a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td +++ b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td @@ -344,7 +344,7 @@ def AffineForOp : Affine_Op<"for", let hasCanonicalizer = 1; let hasCustomAssemblyFormat = 1; let hasFolder = 1; - let hasVerifier = 1; + let hasRegionVerifier = 1; } def AffineIfOp : Affine_Op<"if", diff --git a/mlir/include/mlir/Dialect/Async/IR/AsyncOps.td b/mlir/include/mlir/Dialect/Async/IR/AsyncOps.td index f2138131e1f8..385acdaecf55 100644 --- a/mlir/include/mlir/Dialect/Async/IR/AsyncOps.td +++ b/mlir/include/mlir/Dialect/Async/IR/AsyncOps.td @@ -84,7 +84,7 @@ def Async_ExecuteOp : let hasCustomAssemblyFormat = 1; let skipDefaultBuilders = 1; - let hasVerifier = 1; + let hasRegionVerifier = 1; let builders = [ OpBuilder<(ins "TypeRange":$resultTypes, "ValueRange":$dependencies, "ValueRange":$operands, diff --git a/mlir/include/mlir/Dialect/GPU/GPUOps.td b/mlir/include/mlir/Dialect/GPU/GPUOps.td index ecddae6b10d0..91e5185250a7 100644 --- a/mlir/include/mlir/Dialect/GPU/GPUOps.td +++ b/mlir/include/mlir/Dialect/GPU/GPUOps.td @@ -556,7 +556,7 @@ def GPU_LaunchOp : GPU_Op<"launch", [AutomaticAllocationScope]>, let hasCanonicalizer = 1; let hasCustomAssemblyFormat = 1; - let hasVerifier = 1; + let hasRegionVerifier = 1; } def GPU_PrintfOp : GPU_Op<"printf", [MemoryEffects<[MemWrite]>]>, @@ -677,7 +677,7 @@ def GPU_AllReduceOp : GPU_Op<"all_reduce", let regions = (region AnyRegion:$body); let assemblyFormat = [{ custom<AllReduceOperation>($op) $value $body attr-dict `:` functional-type(operands, results) }]; - let hasVerifier = 1; + let hasRegionVerifier = 1; } def GPU_ShuffleOpXor : I32EnumAttrCase<"XOR", 0, "xor">; diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td index 73efeadbdbda..a69a5c62902d 100644 --- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td +++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td @@ -1140,6 +1140,7 @@ def LLVM_GlobalOp : LLVM_Op<"mlir.global", let hasCustomAssemblyFormat = 1; let hasVerifier = 1; + let hasRegionVerifier = 1; } def LLVM_GlobalCtorsOp : LLVM_Op<"mlir.global_ctors", [ @@ -1277,6 +1278,7 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [ let hasCustomAssemblyFormat = 1; let hasVerifier = 1; + let hasRegionVerifier = 1; } def LLVM_NullOp diff --git a/mlir/include/mlir/Dialect/Linalg/IR/LinalgInterfaces.td b/mlir/include/mlir/Dialect/Linalg/IR/LinalgInterfaces.td index 4ac6bb78653c..a63e653f2a29 100644 --- a/mlir/include/mlir/Dialect/Linalg/IR/LinalgInterfaces.td +++ b/mlir/include/mlir/Dialect/Linalg/IR/LinalgInterfaces.td @@ -34,6 +34,7 @@ def LinalgContractionOpInterface : OpInterface<"ContractionOpInterface"> { }]; let cppNamespace = "::mlir::linalg"; let verify = [{ return detail::verifyContractionInterface($_op); }]; + let verifyWithRegions = 1; let methods = [ InterfaceMethod< /*desc=*/"Returns the left-hand side operand.", @@ -1136,6 +1137,7 @@ def LinalgStructuredInterface : OpInterface<"LinalgOp"> { }]; let verify = [{ return detail::verifyStructuredOpInterface($_op); }]; + let verifyWithRegions = 1; } #endif // LINALG_IR_LINALGINTERFACES diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td index 08867e00d4c8..aedd1e5fe95d 100644 --- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td +++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td @@ -204,6 +204,7 @@ def SectionsOp : OpenMP_Op<"sections", [AttrSizedOperandSegments]> { }]; let hasVerifier = 1; + let hasRegionVerifier = 1; } //===----------------------------------------------------------------------===// @@ -437,7 +438,8 @@ def CriticalDeclareOp : OpenMP_Op<"critical.declare", [Symbol]> { } -def CriticalOp : OpenMP_Op<"critical"> { +def CriticalOp : OpenMP_Op<"critical", + [DeclareOpInterfaceMethods<SymbolUserOpInterface>]> { let summary = "critical construct"; let description = [{ The critical construct imposes a restriction on the associated structured @@ -451,7 +453,6 @@ def CriticalOp : OpenMP_Op<"critical"> { let assemblyFormat = [{ (`(` $name^ `)`)? $region attr-dict }]; - let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -671,6 +672,7 @@ def AtomicUpdateOp : OpenMP_Op<"atomic.update", $x `:` type($x) $region attr-dict }]; let hasVerifier = 1; + let hasRegionVerifier = 1; } def AtomicCaptureOp : OpenMP_Op<"atomic.capture", @@ -717,7 +719,7 @@ def AtomicCaptureOp : OpenMP_Op<"atomic.capture", |`hint` `(` custom<SynchronizationHint>($hint_val) `)`) $region attr-dict }]; - let hasVerifier = 1; + let hasRegionVerifier = 1; } //===----------------------------------------------------------------------===// @@ -771,7 +773,7 @@ def ReductionDeclareOp : OpenMP_Op<"reduction.declare", [Symbol]> { return atomicReductionRegion().front().getArgument(0).getType(); } }]; - let hasVerifier = 1; + let hasRegionVerifier = 1; } //===----------------------------------------------------------------------===// diff --git a/mlir/include/mlir/Dialect/PDL/IR/PDLOps.td b/mlir/include/mlir/Dialect/PDL/IR/PDLOps.td index 56100641281d..eec878ba57b5 100644 --- a/mlir/include/mlir/Dialect/PDL/IR/PDLOps.td +++ b/mlir/include/mlir/Dialect/PDL/IR/PDLOps.td @@ -453,7 +453,7 @@ def PDL_PatternOp : PDL_Op<"pattern", [ /// Returns the rewrite operation of this pattern. RewriteOp getRewriter(); }]; - let hasVerifier = 1; + let hasRegionVerifier = 1; } //===----------------------------------------------------------------------===// @@ -632,7 +632,7 @@ def PDL_RewriteOp : PDL_Op<"rewrite", [ ($body^)? attr-dict-with-keyword }]; - let hasVerifier = 1; + let hasRegionVerifier = 1; } //===----------------------------------------------------------------------===// diff --git a/mlir/include/mlir/Dialect/SCF/SCFOps.td b/mlir/include/mlir/Dialect/SCF/SCFOps.td index 8ad75ed85398..b9cb09de04ea 100644 --- a/mlir/include/mlir/Dialect/SCF/SCFOps.td +++ b/mlir/include/mlir/Dialect/SCF/SCFOps.td @@ -308,6 +308,7 @@ def ForOp : SCF_Op<"for", let hasCanonicalizer = 1; let hasCustomAssemblyFormat = 1; let hasVerifier = 1; + let hasRegionVerifier = 1; } def IfOp : SCF_Op<"if", @@ -535,7 +536,7 @@ def ReduceOp : SCF_Op<"reduce", [HasParent<"ParallelOp">]> { let arguments = (ins AnyType:$operand); let hasCustomAssemblyFormat = 1; let regions = (region SizedRegion<1>:$reductionOperator); - let hasVerifier = 1; + let hasRegionVerifier = 1; } def ReduceReturnOp : @@ -688,7 +689,7 @@ def WhileOp : SCF_Op<"while", let hasCanonicalizer = 1; let hasCustomAssemblyFormat = 1; - let hasVerifier = 1; + let hasRegionVerifier = 1; } def YieldOp : SCF_Op<"yield", [NoSideEffect, ReturnLike, Terminator, diff --git a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td index b65843214caa..3cafd3c4f552 100644 --- a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td +++ b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td @@ -309,6 +309,9 @@ def SPV_LoopOp : SPV_Op<"mlir.loop", [InFunctionScope]> { let hasOpcode = 0; let autogenSerialization = 0; + + let hasVerifier = 0; + let hasRegionVerifier = 1; } // ----- @@ -470,6 +473,9 @@ def SPV_SelectionOp : SPV_Op<"mlir.selection", [InFunctionScope]> { let autogenSerialization = 0; let hasCanonicalizer = 1; + + let hasVerifier = 0; + let hasRegionVerifier = 1; } #endif // MLIR_DIALECT_SPIRV_IR_CONTROLFLOW_OPS diff --git a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVStructureOps.td b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVStructureOps.td index 4201e0ee0933..709db1fbdc46 100644 --- a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVStructureOps.td +++ b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVStructureOps.td @@ -516,6 +516,9 @@ def SPV_ModuleOp : SPV_Op<"module", static StringRef getVCETripleAttrName() { return "vce_triple"; } }]; + + let hasVerifier = 0; + let hasRegionVerifier = 1; } // ----- @@ -749,6 +752,9 @@ def SPV_SpecConstantOperationOp : SPV_Op<"SpecConstantOperation", [ let hasOpcode = 0; let autogenSerialization = 0; + + let hasVerifier = 0; + let hasRegionVerifier = 1; } // ----- diff --git a/mlir/include/mlir/Dialect/Tensor/IR/TensorOps.td b/mlir/include/mlir/Dialect/Tensor/IR/TensorOps.td index a53f3e7e5ca3..c445061c160b 100644 --- a/mlir/include/mlir/Dialect/Tensor/IR/TensorOps.td +++ b/mlir/include/mlir/Dialect/Tensor/IR/TensorOps.td @@ -408,6 +408,7 @@ def Tensor_GenerateOp : Tensor_Op<"generate", let hasCanonicalizer = 1; let hasVerifier = 1; + let hasRegionVerifier = 1; } //===----------------------------------------------------------------------===// @@ -983,6 +984,7 @@ def Tensor_PadOp : Tensor_Op<"pad", [AttrSizedOperandSegments, NoSideEffect, let hasCanonicalizer = 1; let hasFolder = 1; let hasVerifier = 1; + let hasRegionVerifier = 1; } //===----------------------------------------------------------------------===// diff --git a/mlir/include/mlir/IR/OpBase.td b/mlir/include/mlir/IR/OpBase.td index 3310569a8fff..e073048e24c2 100644 --- a/mlir/include/mlir/IR/OpBase.td +++ b/mlir/include/mlir/IR/OpBase.td @@ -2073,9 +2073,11 @@ def ResultsBroadcastableShape : // X op Y == Y op X def Commutative : NativeOpTrait<"IsCommutative">; // op op X == op X (unary) / X op X == X (binary) -def Idempotent : NativeOpTrait<"IsIdempotent">; +// FIXME: Idempotent should depend on SameOperandsAndResultType +def Idempotent : NativeOpTrait<"IsIdempotent">; // op op X == X -def Involution : NativeOpTrait<"IsInvolution">; +// FIXME: Involution should depend on SameOperandsAndResultType +def Involution : NativeOpTrait<"IsInvolution">; // Op behaves like a constant. def ConstantLike : NativeOpTrait<"ConstantLike">; // Op is isolated from above. @@ -2103,11 +2105,11 @@ def MemRefsNormalizable : NativeOpTrait<"MemRefsNormalizable">; // Op is elementwise on tensor/vector operands and results. def Elementwise : NativeOpTrait<"Elementwise">; // Elementwise op can be applied to scalars instead tensor/vector operands. -def Scalarizable : NativeOpTrait<"Scalarizable">; +def Scalarizable : NativeOpTrait<"Scalarizable", [Elementwise]>; // Elementwise op can be applied to all-vector operands. -def Vectorizable : NativeOpTrait<"Vectorizable">; +def Vectorizable : NativeOpTrait<"Vectorizable", [Elementwise]>; // Elementwise op can be applied to all-tensor operands. -def Tensorizable : NativeOpTrait<"Tensorizable">; +def Tensorizable : NativeOpTrait<"Tensorizable", [Elementwise]>; // Group together `Elementwise`, `Scalarizable`, `Vectorizable`, and // `Tensorizable` for convenience. @@ -2489,7 +2491,9 @@ class Op<Dialect dialect, string mnemonic, list<Trait> props = []> { // verified (aside from those verified by other ODS constructs). If set to `1`, // an additional `LogicalResult verify()` declaration will be generated on the // operation class. The operation should implement this method and verify the - // additional necessary invariants. + // additional necessary invariants. This verifier shouldn't access any nested + // operations because those operations may ill-formed. Use the + // `hasRegionVerifier` below instead. bit hasVerifier = 0; // A bit indicating if the operation has additional invariants that need to diff --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h index 268b666e1366..c814b2bcbc7f 100644 --- a/mlir/include/mlir/IR/OpDefinition.h +++ b/mlir/include/mlir/IR/OpDefinition.h @@ -920,7 +920,7 @@ struct SingleBlockImplicitTerminator { /// The type of the operation used as the implicit terminator type. using ImplicitTerminatorOpT = TerminatorOpType; - static LogicalResult verifyTrait(Operation *op) { + static LogicalResult verifyRegionTrait(Operation *op) { if (failed(Base::verifyTrait(op))) return failure(); for (unsigned i = 0, e = op->getNumRegions(); i < e; ++i) { @@ -1218,7 +1218,7 @@ template <typename ConcreteType> class IsIsolatedFromAbove : public TraitBase<ConcreteType, IsIsolatedFromAbove> { public: - static LogicalResult verifyTrait(Operation *op) { + static LogicalResult verifyRegionTrait(Operation *op) { return impl::verifyIsIsolatedFromAbove(op); } }; @@ -1262,7 +1262,7 @@ struct HasParent { class Impl : public TraitBase<ConcreteType, Impl> { public: static LogicalResult verifyTrait(Operation *op) { - if (llvm::isa<ParentOpTypes...>(op->getParentOp())) + if (llvm::isa_and_nonnull<ParentOpTypes...>(op->getParentOp())) return success(); return op->emitOpError() diff --git a/mlir/include/mlir/IR/SymbolTable.h b/mlir/include/mlir/IR/SymbolTable.h index f1e443ab7a2e..86ca3b173b37 100644 --- a/mlir/include/mlir/IR/SymbolTable.h +++ b/mlir/include/mlir/IR/SymbolTable.h @@ -337,7 +337,7 @@ namespace OpTrait { template <typename ConcreteType> class SymbolTable : public TraitBase<ConcreteType, SymbolTable> { public: - static LogicalResult verifyTrait(Operation *op) { + static LogicalResult verifyRegionTrait(Operation *op) { return ::mlir::detail::verifySymbolTable(op); } diff --git a/mlir/include/mlir/Interfaces/InferTypeOpInterface.td b/mlir/include/mlir/Interfaces/InferTypeOpInterface.td index fcf3d22d4ff6..eb1860656a36 100644 --- a/mlir/include/mlir/Interfaces/InferTypeOpInterface.td +++ b/mlir/include/mlir/Interfaces/InferTypeOpInterface.td @@ -64,6 +64,8 @@ def InferTypeOpInterface : OpInterface<"InferTypeOpInterface"> { >, ]; + // Inferring result types may need to access the region operations. + let verifyWithRegions = 1; let verify = [{ return detail::verifyInferredResultTypes($_op); }]; diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp index c2952069b774..3b288bed37bb 100644 --- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp +++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp @@ -1304,7 +1304,7 @@ void AffineForOp::build(OpBuilder &builder, OperationState &result, int64_t lb, bodyBuilder); } -LogicalResult AffineForOp::verify() { +LogicalResult AffineForOp::verifyRegions() { // Check that the body defines as single block argument for the induction // variable. auto *body = getBody(); diff --git a/mlir/lib/Dialect/Async/IR/Async.cpp b/mlir/lib/Dialect/Async/IR/Async.cpp index 541c69ec58bc..546a39f38f03 100644 --- a/mlir/lib/Dialect/Async/IR/Async.cpp +++ b/mlir/lib/Dialect/Async/IR/Async.cpp @@ -236,7 +236,7 @@ ParseResult ExecuteOp::parse(OpAsmParser &parser, OperationState &result) { return success(); } -LogicalResult ExecuteOp::verify() { +LogicalResult ExecuteOp::verifyRegions() { // Unwrap async.execute value operands types. auto unwrappedTypes = llvm::map_range(operands(), [](Value operand) { return operand.getType().cast<ValueType>().getValueType(); diff --git a/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp b/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp index 24deaad63221..a8ff898b3a91 100644 --- a/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp +++ b/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp @@ -275,7 +275,7 @@ LogicalResult GPUDialect::verifyOperationAttribute(Operation *op, return walkResult.wasInterrupted() ? failure() : success(); } -LogicalResult gpu::AllReduceOp::verify() { +LogicalResult gpu::AllReduceOp::verifyRegions() { if (body().empty() != op().hasValue()) return emitError("expected either an op attribute or a non-empty body"); if (!body().empty()) { @@ -407,7 +407,7 @@ KernelDim3 LaunchOp::getBlockSizeOperandValues() { return KernelDim3{getOperand(3), getOperand(4), getOperand(5)}; } -LogicalResult LaunchOp::verify() { +LogicalResult LaunchOp::verifyRegions() { // Kernel launch takes kNumConfigOperands leading operands for grid/block // sizes and transforms them into kNumConfigRegionAttributes region arguments // for block/thread identifiers and grid/block sizes. diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp index d0963a49f79b..a816fb83fa2d 100644 --- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp +++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp @@ -1795,26 +1795,6 @@ LogicalResult GlobalOp::verify() { "attribute"); } - if (Block *b = getInitializerBlock()) { - ReturnOp ret = cast<ReturnOp>(b->getTerminator()); - if (ret.operand_type_begin() == ret.operand_type_end()) - return emitOpError("initializer region cannot return void"); - if (*ret.operand_type_begin() != getType()) - return emitOpError("initializer region type ") - << *ret.operand_type_begin() << " does not match global type " - << getType(); - - for (Operation &op : *b) { - auto iface = dyn_cast<MemoryEffectOpInterface>(op); - if (!iface || !iface.hasNoEffect()) - return op.emitError() - << "ops with side effects not allowed in global initializers"; - } - - if (getValueOrNull()) - return emitOpError("cannot have both initializer value and region"); - } - if (getLinkage() == Linkage::Common) { if (Attribute value = getValueOrNull()) { if (!isZeroAttribute(value)) { @@ -1843,6 +1823,30 @@ LogicalResult GlobalOp::verify() { return success(); } +LogicalResult GlobalOp::verifyRegions() { + if (Block *b = getInitializerBlock()) { + ReturnOp ret = cast<ReturnOp>(b->getTerminator()); + if (ret.operand_type_begin() == ret.operand_type_end()) + return emitOpError("initializer region cannot return void"); + if (*ret.operand_type_begin() != getType()) + return emitOpError("initializer region type ") + << *ret.operand_type_begin() << " does not match global type " + << getType(); + + for (Operation &op : *b) { + auto iface = dyn_cast<MemoryEffectOpInterface>(op); + if (!iface || !iface.hasNoEffect()) + return op.emitError() + << "ops with side effects not allowed in global initializers"; + } + + if (getValueOrNull()) + return emitOpError("cannot have both initializer value and region"); + } + + return success(); +} + //===----------------------------------------------------------------------===// // LLVM::GlobalCtorsOp //===----------------------------------------------------------------------===// @@ -2123,7 +2127,6 @@ LogicalResult LLVMFuncOp::verifyType() { // - functions don't have 'common' linkage // - external functions have 'external' or 'extern_weak' linkage; // - vararg is (currently) only supported for external functions; -// - entry block arguments are of LLVM types and match the function signature. LogicalResult LLVMFuncOp::verify() { if (getLinkage() == LLVM::Linkage::Common) return emitOpError() << "functions cannot have '" @@ -2152,6 +2155,15 @@ LogicalResult LLVMFuncOp::verify() { if (isVarArg()) return emitOpError("only external functions can be variadic"); + return success(); +} + +/// Verifies LLVM- and implementation-specific properties of the LLVM func Op: +/// - entry block arguments are of LLVM types and match the function signature. +LogicalResult LLVMFuncOp::verifyRegions() { + if (isExternal()) + return success(); + unsigned numArguments = getType().getNumParams(); Block &entryBlock = front(); for (unsigned i = 0; i < numArguments; ++i) { diff --git a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp index 8880c16b8ccd..1e8c29f58417 100644 --- a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp +++ b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp @@ -1207,7 +1207,7 @@ LogicalResult linalg::YieldOp::verify() { return emitOpError("expected single non-empty parent region"); if (auto linalgOp = dyn_cast<LinalgOp>(parentOp)) - return verifyYield(*this, cast<LinalgOp>(parentOp)); + return verifyYield(*this, linalgOp); return emitOpError("expected parent op with LinalgOp interface"); } diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp index b05fa8a92f83..15e9cbcb35b3 100644 --- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp +++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp @@ -369,6 +369,8 @@ static LogicalResult verifyReductionVarList(Operation *op, return success(); } + // TODO: The followings should be done in + // SymbolUserOpInterface::verifySymbolUses. DenseSet<Value> accumulators; for (auto args : llvm::zip(reductionVars, *reductions)) { Value accum = std::get<0>(args); @@ -719,6 +721,10 @@ LogicalResult SectionsOp::verify() { return emitError( "expected equal sizes for allocate and allocator variables"); + return verifyReductionVarList(*this, reductions(), reduction_vars()); +} + +LogicalResult SectionsOp::verifyRegions() { for (auto &inst : *region().begin()) { if (!(isa<SectionOp>(inst) || isa<TerminatorOp>(inst))) { return emitOpError() @@ -726,7 +732,7 @@ LogicalResult SectionsOp::verify() { } } - return verifyReductionVarList(*this, reductions(), reduction_vars()); + return success(); } /// Parses an OpenMP Workshare Loop operation @@ -851,7 +857,7 @@ static void printAtomicReductionRegion(OpAsmPrinter &printer, printer.printRegion(region); } -LogicalResult ReductionDeclareOp::verify() { +LogicalResult ReductionDeclareOp::verifyRegions() { if (initializerRegion().empty()) return emitOpError() << "expects non-empty initializer region"; Block &initializerEntryBlock = initializerRegion().front(); @@ -941,10 +947,11 @@ LogicalResult CriticalDeclareOp::verify() { return verifySynchronizationHint(*this, hint_val()); } -LogicalResult CriticalOp::verify() { +LogicalResult +CriticalOp::verifySymbolUses(SymbolTableCollection &symbol_table) { if (nameAttr()) { SymbolRefAttr symbolRef = nameAttr(); - auto decl = SymbolTable::lookupNearestSymbolFrom<CriticalDeclareOp>( + auto decl = symbol_table.lookupNearestSymbolFrom<CriticalDeclareOp>( *this, symbolRef); if (!decl) { return emitOpError() << "expected symbol reference " << symbolRef @@ -1038,15 +1045,19 @@ LogicalResult AtomicUpdateOp::verify() { } } - if (region().getNumArguments() != 1) - return emitError("the region must accept exactly one argument"); - if (x().getType().cast<PointerLikeType>().getElementType() != region().getArgument(0).getType()) { return emitError("the type of the operand must be a pointer type whose " "element type is the same as that of the region argument"); } + return success(); +} + +LogicalResult AtomicUpdateOp::verifyRegions() { + if (region().getNumArguments() != 1) + return emitError("the region must accept exactly one argument"); + if (region().front().getOperations().size() < 2) return emitError() << "the update region must have at least two operations " "(binop and terminator)"; @@ -1064,7 +1075,7 @@ LogicalResult AtomicUpdateOp::verify() { // Verifier for AtomicCaptureOp //===----------------------------------------------------------------------===// -LogicalResult AtomicCaptureOp::verify() { +LogicalResult AtomicCaptureOp::verifyRegions() { Block::OpListType &ops = region().front().getOperations(); if (ops.size() != 3) return emitError() diff --git a/mlir/lib/Dialect/PDL/IR/PDL.cpp b/mlir/lib/Dialect/PDL/IR/PDL.cpp index 7e655e124771..c11f2f6ca9bc 100644 --- a/mlir/lib/Dialect/PDL/IR/PDL.cpp +++ b/mlir/lib/Dialect/PDL/IR/PDL.cpp @@ -269,7 +269,7 @@ bool OperationOp::hasTypeInference() { // pdl::PatternOp //===----------------------------------------------------------------------===// -LogicalResult PatternOp::verify() { +LogicalResult PatternOp::verifyRegions() { Region &body = getBodyRegion(); Operation *term = body.front().getTerminator(); auto rewriteOp = dyn_cast<RewriteOp>(term); @@ -402,7 +402,7 @@ LogicalResult ResultsOp::verify() { // pdl::RewriteOp //===----------------------------------------------------------------------===// -LogicalResult RewriteOp::verify() { +LogicalResult RewriteOp::verifyRegions() { Region &rewriteRegion = body(); // Handle the case where the rewrite is external. diff --git a/mlir/lib/Dialect/SCF/SCF.cpp b/mlir/lib/Dialect/SCF/SCF.cpp index aced058682f6..4e7ff0829c8d 100644 --- a/mlir/lib/Dialect/SCF/SCF.cpp +++ b/mlir/lib/Dialect/SCF/SCF.cpp @@ -282,6 +282,19 @@ LogicalResult ForOp::verify() { if (cst.value() <= 0) return emitOpError("constant step operand must be positive"); + auto opNumResults = getNumResults(); + if (opNumResults == 0) + return success(); + // If ForOp defines values, check that the number and types of + // the defined values match ForOp initial iter operands and backedge + // basic block arguments. + if (getNumIterOperands() != opNumResults) + return emitOpError( + "mismatch in number of loop-carried values and defined values"); + return success(); +} + +LogicalResult ForOp::verifyRegions() { // Check that the body defines as single block argument for the induction // variable. auto *body = getBody(); @@ -293,15 +306,11 @@ LogicalResult ForOp::verify() { auto opNumResults = getNumResults(); if (opNumResults == 0) return success(); - // If ForOp defines values, check that the number and types of - // the defined values match ForOp initial iter operands and backedge - // basic block arguments. - if (getNumIterOperands() != opNumResults) - return emitOpError( - "mismatch in number of loop-carried values and defined values"); + if (getNumRegionIterArgs() != opNumResults) return emitOpError( "mismatch in number of basic block args and defined values"); + auto iterOperands = getIterOperands(); auto iterArgs = getRegionIterArgs(); auto opResults = getResults(); @@ -2130,7 +2139,7 @@ void ReduceOp::build( body->getArgument(1)); } -LogicalResult ReduceOp::verify() { +LogicalResult ReduceOp::verifyRegions() { // The region of a ReduceOp has two arguments of the same type as its operand. auto type = getOperand().getType(); Block &block = getReductionOperator().front(); @@ -2333,7 +2342,7 @@ static TerminatorTy verifyAndGetTerminator(scf::WhileOp op, Region ®ion, return nullptr; } -LogicalResult scf::WhileOp::verify() { +LogicalResult scf::WhileOp::verifyRegions() { auto beforeTerminator = verifyAndGetTerminator<scf::ConditionOp>( *this, getBefore(), "expects the 'before' region to terminate with 'scf.condition'"); diff --git a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp index 6a5d7c7c194d..a7f3ed7b5479 100644 --- a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp +++ b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp @@ -2935,7 +2935,7 @@ static inline bool hasOneBranchOpTo(Block &srcBlock, Block &dstBlock) { return branchOp && branchOp.getSuccessor() == &dstBlock; } -LogicalResult spirv::LoopOp::verify() { +LogicalResult spirv::LoopOp::verifyRegions() { auto *op = getOperation(); // We need to verify that the blocks follow the following layout: @@ -3072,6 +3072,7 @@ LogicalResult spirv::MergeOp::verify() { return emitOpError( "expected parent op to be 'spv.mlir.selection' or 'spv.mlir.loop'"); + // TODO: This check should be done in `verifyRegions` of parent op. Block &parentLastBlock = (*this)->getParentRegion()->back(); if (getOperation() != parentLastBlock.getTerminator()) return emitOpError("can only be used in the last block of " @@ -3173,7 +3174,7 @@ void spirv::ModuleOp::print(OpAsmPrinter &printer) { printer.printRegion(getRegion()); } -LogicalResult spirv::ModuleOp::verify() { +LogicalResult spirv::ModuleOp::verifyRegions() { Dialect *dialect = (*this)->getDialect(); DenseMap<std::pair<spirv::FuncOp, spirv::ExecutionModel>, spirv::EntryPointOp> entryPoints; @@ -3322,7 +3323,7 @@ void spirv::SelectionOp::print(OpAsmPrinter &printer) { /*printBlockTerminators=*/true); } -LogicalResult spirv::SelectionOp::verify() { +LogicalResult spirv::SelectionOp::verifyRegions() { auto *op = getOperation(); // We need to verify that the blocks follow the following layout: @@ -4106,7 +4107,7 @@ void spirv::SpecConstantOperationOp::print(OpAsmPrinter &printer) { printer.printGenericOp(&body().front().front()); } -LogicalResult spirv::SpecConstantOperationOp::verify() { +LogicalResult spirv::SpecConstantOperationOp::verifyRegions() { Block &block = getRegion().getBlocks().front(); if (block.getOperations().size() != 2) diff --git a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp index e93d619bce74..190c4b1b6bc9 100644 --- a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp +++ b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp @@ -533,6 +533,11 @@ LogicalResult GenerateOp::verify() { return emitError("must have as many index operands as dynamic extents " "in the result type"); + return success(); +} + +LogicalResult GenerateOp::verifyRegions() { + RankedTensorType resultTy = getType().cast<RankedTensorType>(); // Ensure that region arguments span the index space. if (!llvm::all_of(body().getArgumentTypes(), [](Type ty) { return ty.isIndex(); })) @@ -1749,8 +1754,12 @@ LogicalResult PadOp::verify() { << expectedType; } + return success(); +} + +LogicalResult PadOp::verifyRegions() { auto ®ion = getRegion(); - unsigned rank = resultType.getRank(); + unsigned rank = result().getType().cast<RankedTensorType>().getRank(); Block &block = region.front(); if (block.getNumArguments() != rank) return emitError("expected the block to have ") << rank << " arguments"; diff --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp index ea68f11e126a..19fb180676fa 100644 --- a/mlir/lib/IR/Operation.cpp +++ b/mlir/lib/IR/Operation.cpp @@ -1088,12 +1088,6 @@ LogicalResult OpTrait::impl::verifyIsIsolatedFromAbove(Operation *isolatedOp) { while (!pendingRegions.empty()) { for (Operation &op : pendingRegions.pop_back_val()->getOps()) { for (Value operand : op.getOperands()) { - // operand should be non-null here if the IR is well-formed. But - // we don't assert here as this function is called from the verifier - // and so could be called on invalid IR. - if (!operand) - return op.emitOpError("operation's operand is null"); - // Check that any value that is used by an operation is defined in the // same region as either an operation result. auto *operandRegion = operand.getParentRegion(); diff --git a/mlir/test/Dialect/GPU/invalid.mlir b/mlir/test/Dialect/GPU/invalid.mlir index cdaef0196929..07dd173d8df7 100644 --- a/mlir/test/Dialect/GPU/invalid.mlir +++ b/mlir/test/Dialect/GPU/invalid.mlir @@ -15,7 +15,7 @@ func @no_region_attrs(%sz : index) { "gpu.launch"(%sz, %sz, %sz, %sz, %sz, %sz) ({ ^bb1(%bx: index, %by: index, %bz: index, %tx: index, %ty: index, %tz: index): - gpu.return + gpu.terminator }) : (index, index, index, index, index, index) -> () return } @@ -26,8 +26,9 @@ func @launch_requires_gpu_return(%sz : index) { // @expected-note@+1 {{in 'gpu.launch' body region}} gpu.launch blocks(%bx, %by, %bz) in (%sbx = %sz, %sby = %sz, %sbz = %sz) threads(%tx, %ty, %tz) in (%stx = %sz, %sty = %sz, %stz = %sz) { - // @expected-error@+1 {{expected 'gpu.terminator' or a terminator with successors}} - return + // @expected-error@+2 {{expected 'gpu.terminator' or a terminator with successors}} + %one = arith.constant 1 : i32 + "gpu.yield"(%one) : (i32) -> () } return } @@ -293,7 +294,7 @@ func @reduce_incorrect_yield(%arg0 : f32) { // expected-error@+1 {{expected gpu.yield op in region}} %res = gpu.all_reduce %arg0 { ^bb(%lhs : f32, %rhs : f32): - return + "test.finish" () : () -> () } : (f32) -> (f32) return } diff --git a/mlir/test/Dialect/LLVMIR/global.mlir b/mlir/test/Dialect/LLVMIR/global.mlir index 84f1d9e93584..07e5ac0f061a 100644 --- a/mlir/test/Dialect/LLVMIR/global.mlir +++ b/mlir/test/Dialect/LLVMIR/global.mlir @@ -172,8 +172,7 @@ llvm.func @bar() { // ----- -// expected-error @+2 {{'llvm.mlir.global' op expects regions to end with 'llvm.return', found 'llvm.mlir.constant'}} -// expected-note @+1 {{in custom textual format, the absence of terminator implies 'llvm.return'}} +// expected-error @+2 {{block with no terminator}} llvm.mlir.global internal @g() : i64 { %c = llvm.mlir.constant(42 : i64) : i64 } diff --git a/mlir/test/Dialect/Linalg/invalid.mlir b/mlir/test/Dialect/Linalg/invalid.mlir index f220d8ecf3a5..132d14d6c415 100644 --- a/mlir/test/Dialect/Linalg/invalid.mlir +++ b/mlir/test/Dialect/Linalg/invalid.mlir @@ -202,7 +202,7 @@ func @generic_empty_region(%arg0: memref<f32>) { // ----- func @generic_mismatched_num_arguments(%arg0: memref<f32>) { - // expected-error @+1 {{expected as many non-induction variable region arguments as the number of input/output operands}} + // expected-error @+6 {{'linalg.yield' op expected number of yield values (2) to match the number of operands of the enclosing LinalgOp (1)}} linalg.generic { indexing_maps = [ affine_map<() -> ()>, affine_map<() -> ()> ], iterator_types = []} @@ -215,7 +215,7 @@ func @generic_mismatched_num_arguments(%arg0: memref<f32>) { // ----- func @generic_shaped_operand_block_arg_type(%arg0: memref<f32>) { - // expected-error @+1 {{expected type of bb argument #0 ('i1') to match element or self type of the corresponding operand ('f32')}} + // expected-error @+6 {{'linalg.yield' op type of yield operand 1 ('i1') doesn't match the element type of the enclosing linalg.generic op ('f32')}} linalg.generic { indexing_maps = [ affine_map<() -> ()> ], iterator_types = []} @@ -228,7 +228,7 @@ func @generic_shaped_operand_block_arg_type(%arg0: memref<f32>) { // ----- func @generic_scalar_operand_block_arg_type(%arg0: tensor<f32>) { - // expected-error @+1 {{expected type of bb argument #0 ('i1') to match element or self type of the corresponding operand ('f32')}} + // expected-error @+6 {{'linalg.yield' op type of yield operand 1 ('i1') doesn't match the element type of the enclosing linalg.generic op ('f32')}} linalg.generic { indexing_maps = [ affine_map<() -> ()> ], iterator_types = []} @@ -284,15 +284,14 @@ func @generic_result_tensor_type(%arg0: memref<?xf32, affine_map<(i)[off]->(off // ----- -func @generic(%arg0: memref<?x?xi4>) { - // expected-error @+2 {{op expects regions to end with 'linalg.yield', found 'arith.addf'}} - // expected-note @+1 {{in custom textual format, the absence of terminator implies 'linalg.yield'}} +func @generic(%arg0: memref<?x?xf32>) { + // expected-error @+6 {{block with no terminator, has %0 = "arith.addf"(%arg1, %arg1) : (f32, f32) -> f32}} linalg.generic { indexing_maps = [ affine_map<(i, j) -> (i, j)> ], iterator_types = ["parallel", "parallel"]} - outs(%arg0 : memref<?x?xi4>) { - ^bb(%0: i4) : - %1 = arith.addf %0, %0: i4 + outs(%arg0 : memref<?x?xf32>) { + ^bb(%0: f32) : + %1 = arith.addf %0, %0: f32 } return } diff --git a/mlir/test/Dialect/PDL/invalid.mlir b/mlir/test/Dialect/PDL/invalid.mlir index 2c630269f80d..f8f641e8d298 100644 --- a/mlir/test/Dialect/PDL/invalid.mlir +++ b/mlir/test/Dialect/PDL/invalid.mlir @@ -159,7 +159,7 @@ pdl.pattern : benefit(1) { // expected-error@below {{expected body to terminate with `pdl.rewrite`}} pdl.pattern : benefit(1) { // expected-note@below {{see terminator defined here}} - return + "test.finish" () : () -> () } // ----- diff --git a/mlir/test/Dialect/SCF/invalid.mlir b/mlir/test/Dialect/SCF/invalid.mlir index 9b2fe2ff218a..941a2a299440 100644 --- a/mlir/test/Dialect/SCF/invalid.mlir +++ b/mlir/test/Dialect/SCF/invalid.mlir @@ -241,7 +241,7 @@ func @reduce_empty_block(%arg0 : index, %arg1 : f32) { %zero = arith.constant 0.0 : f32 %res = scf.parallel (%i0) = (%arg0) to (%arg0) step (%arg0) init (%zero) -> f32 { - // expected-error@+1 {{the block inside reduce should not be empty}} + // expected-error@+1 {{empty block: expect at least a terminator}} scf.reduce(%arg1) : f32 { ^bb0(%lhs : f32, %rhs : f32): } @@ -289,7 +289,7 @@ func @reduce_wrong_terminator(%arg0 : index, %arg1 : f32) { // expected-error@+1 {{the block inside reduce should be terminated with a 'scf.reduce.return' op}} scf.reduce(%arg1) : f32 { ^bb0(%lhs : f32, %rhs : f32): - scf.yield + "test.finish" () : () -> () } } return diff --git a/mlir/test/Dialect/Tensor/invalid.mlir b/mlir/test/Dialect/Tensor/invalid.mlir index 3aaa18e78461..a614d2228e11 100644 --- a/mlir/test/Dialect/Tensor/invalid.mlir +++ b/mlir/test/Dialect/Tensor/invalid.mlir @@ -91,8 +91,7 @@ func @tensor.generate(%m : index, %n : index) func @tensor.generate(%m : index, %n : index) -> tensor<?x3x?xf32> { - // expected-error @+2 {{op expects regions to end with 'tensor.yield', found 'func.return'}} - // expected-note @+1 {{in custom textual format, the absence of terminator implies 'tensor.yield'}} + // expected-error @+4 {{'func.return' op expects parent op 'builtin.func'}} %tnsr = tensor.generate %m, %n { ^bb0(%i : index, %j : index, %k : index): %elem = arith.constant 8.0 : f32 @@ -377,4 +376,4 @@ func @invalid_splat(%v : vector<8xf32>) { // expected-error@+1 {{must be integer/index/float type}} %w = tensor.splat %v : tensor<8xvector<8xf32>> return -}
\ No newline at end of file +} diff --git a/mlir/test/IR/invalid.mlir b/mlir/test/IR/invalid.mlir index bd422f9250c3..a88bfdad75dc 100644 --- a/mlir/test/IR/invalid.mlir +++ b/mlir/test/IR/invalid.mlir @@ -536,8 +536,7 @@ func @return_type_mismatch() -> i32 { func @return_inside_loop() { affine.for %i = 1 to 100 { - // expected-error@-1 {{op expects regions to end with 'affine.yield', found 'func.return'}} - // expected-note@-2 {{in custom textual format, the absence of terminator implies}} + // expected-error@+1 {{'func.return' op expects parent op 'builtin.func'}} return } return diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp index 2c56c92ae127..b7f76367f21b 100644 --- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp +++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp @@ -2297,10 +2297,6 @@ void OpEmitter::genCustomVerifier() { if (def.getValueAsBit("hasVerifier")) { auto *method = opClass.declareMethod("::mlir::LogicalResult", "verify"); ERROR_IF_PRUNED(method, "verify", op); - } else if (def.getValueAsBit("hasRegionVerifier")) { - auto *method = - opClass.declareMethod("::mlir::LogicalResult", "verifyRegions"); - ERROR_IF_PRUNED(method, "verifyRegions", op); } else if (hasCustomVerifyCodeBlock) { auto *method = opClass.addMethod("::mlir::LogicalResult", "verify"); ERROR_IF_PRUNED(method, "verify", op); @@ -2311,6 +2307,12 @@ void OpEmitter::genCustomVerifier() { auto printer = stringInit->getValue().ltrim().rtrim(" \t\v\f\r"); body << " " << tgfmt(printer, &fctx); } + + if (def.getValueAsBit("hasRegionVerifier")) { + auto *method = + opClass.declareMethod("::mlir::LogicalResult", "verifyRegions"); + ERROR_IF_PRUNED(method, "verifyRegions", op); + } } void OpEmitter::genOperandResultVerifier(MethodBody &body, |