From 9445b39673c81e9b5ffeda0f71be0c1476e9f313 Mon Sep 17 00:00:00 2001 From: Chia-hung Duan Date: Fri, 25 Feb 2022 18:17:30 +0000 Subject: [PATCH] [mlir] Support verification order (2/3) This change gives explicit order of verifier execution and adds `hasRegionVerifier` and `verifyWithRegions` to increase the granularity of verifier classification. The orders are as below, 1. InternalOpTrait will be verified first, they can be run independently. 2. `verifyInvariants` which is constructed by ODS, it verifies the type, attributes, .etc. 3. Other Traits/Interfaces that have marked their verifier as `verifyTrait` or `verifyWithRegions=0`. 4. Custom verifier which is defined in the op and has marked `hasVerifier=1` If an operation has regions, then it may have the second phase, 5. Traits/Interfaces that have marked their verifier as `verifyRegionTrait` or `verifyWithRegions=1`. This implies the verifier needs to access the operations in its regions. 6. Custom verifier which is defined in the op and has marked `hasRegionVerifier=1` Note that the second phase will be run after the operations in the region are verified. Based on the verification order, you will be able to avoid verifying duplicate things. Reviewed By: Mogball Differential Revision: https://reviews.llvm.org/D116789 --- mlir/docs/OpDefinitions.md | 37 +++++++++- mlir/docs/Traits.md | 14 ++-- mlir/include/mlir/Dialect/Affine/IR/AffineOps.h | 10 ++- mlir/include/mlir/IR/OpBase.td | 48 +++++++++--- mlir/include/mlir/IR/OpDefinition.h | 63 +++++++++++++++- mlir/include/mlir/IR/OperationSupport.h | 20 +++-- mlir/include/mlir/TableGen/Interfaces.h | 4 + mlir/include/mlir/TableGen/Trait.h | 3 + mlir/lib/Dialect/Affine/IR/AffineOps.cpp | 4 +- mlir/lib/IR/MLIRContext.cpp | 4 +- mlir/lib/IR/Verifier.cpp | 5 ++ mlir/lib/TableGen/Interfaces.cpp | 4 + mlir/lib/TableGen/Trait.cpp | 4 + mlir/test/Dialect/Arithmetic/invalid.mlir | 40 +++++----- mlir/test/Dialect/GPU/invalid.mlir | 2 +- mlir/test/Dialect/LLVMIR/global.mlir | 4 +- mlir/test/Dialect/Linalg/invalid.mlir | 24 +++--- mlir/test/Dialect/Linalg/named-ops.mlir | 4 +- mlir/test/Dialect/SPIRV/IR/bit-ops.mlir | 4 +- mlir/test/Dialect/Shape/invalid.mlir | 4 +- mlir/test/Dialect/traits.mlir | 2 +- mlir/test/IR/invalid-module-op.mlir | 2 +- mlir/test/IR/traits.mlir | 8 +- mlir/test/mlir-tblgen/op-decl-and-defs.td | 2 +- mlir/test/mlir-tblgen/op-interface.td | 19 +++++ mlir/test/mlir-tblgen/types.mlir | 6 +- mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp | 98 ++++++++++++++++++------- mlir/tools/mlir-tblgen/OpInterfacesGen.cpp | 9 ++- 28 files changed, 333 insertions(+), 115 deletions(-) diff --git a/mlir/docs/OpDefinitions.md b/mlir/docs/OpDefinitions.md index e9aa37f..09a3ca5 100644 --- a/mlir/docs/OpDefinitions.md +++ b/mlir/docs/OpDefinitions.md @@ -567,10 +567,39 @@ _additional_ verification, you can use let hasVerifier = 1; ``` -This will generate a `LogicalResult verify()` method declaration on the op class -that can be defined with any additional verification constraints. This method -will be invoked after the auto-generated verification code. The order of trait -verification excluding those of `hasVerifier` should not be relied upon. +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. + +#### Verification Ordering + +The verification of an operation involves several steps, + +1. StructuralOpTrait will be verified first, they can be run independently. +1. `verifyInvariants` which is constructed by ODS, it verifies the type, + attributes, .etc. +1. Other Traits/Interfaces that have marked their verifier as `verifyTrait` or + `verifyWithRegions=0`. +1. Custom verifier which is defined in the op and has marked `hasVerifier=1` + +If an operation has regions, then it may have the second phase, + +1. Traits/Interfaces that have marked their verifier as `verifyRegionTrait` or + `verifyWithRegions=1`. This implies the verifier needs to access the + operations in its regions. +1. Custom verifier which is defined in the op and has marked + `hasRegionVerifier=1` + +Note that the second phase will be run after the operations in the region are +verified. Verifiers further down the order can rely on certain invariants being +verified by a previous verifier and do not need to re-verify them. ### Declarative Assembly Format diff --git a/mlir/docs/Traits.md b/mlir/docs/Traits.md index 065c515..4a6915c 100644 --- a/mlir/docs/Traits.md +++ b/mlir/docs/Traits.md @@ -36,9 +36,12 @@ class MyTrait : public TraitBase { }; ``` -Operation traits may also provide a `verifyTrait` hook, that is called when -verifying the concrete operation. The trait verifiers will currently always be -invoked before the main `Op::verify`. +Operation traits may also provide a `verifyTrait` or `verifyRegionTrait` hook +that is called when verifying the concrete operation. The difference between +these two is that whether the verifier needs to access the regions, if so, the +operations in the regions will be verified before the verification of this +trait. The [verification order](OpDefinitions.md/#verification-ordering) +determines when a verifier will be invoked. ```c++ template @@ -53,8 +56,9 @@ public: ``` Note: It is generally good practice to define the implementation of the -`verifyTrait` hook out-of-line as a free function when possible to avoid -instantiating the implementation for every concrete operation type. +`verifyTrait` or `verifyRegionTrait` hook out-of-line as a free function when +possible to avoid instantiating the implementation for every concrete operation +type. Operation traits may also provide a `foldTrait` hook that is called when folding the concrete operation. The trait folders will only be invoked if the concrete diff --git a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.h b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.h index d2bef9d..96d134d 100644 --- a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.h +++ b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.h @@ -76,7 +76,7 @@ bool isTopLevelValue(Value value); class AffineDmaStartOp : public Op { + OpTrait::OpInvariants, AffineMapAccessInterface::Trait> { public: using Op::Op; static ArrayRef getAttributeNames() { return {}; } @@ -227,7 +227,8 @@ public: static StringRef getOperationName() { return "affine.dma_start"; } static ParseResult parse(OpAsmParser &parser, OperationState &result); void print(OpAsmPrinter &p); - LogicalResult verifyInvariants(); + LogicalResult verifyInvariantsImpl(); + LogicalResult verifyInvariants() { return verifyInvariantsImpl(); } LogicalResult fold(ArrayRef cstOperands, SmallVectorImpl &results); @@ -268,7 +269,7 @@ public: class AffineDmaWaitOp : public Op { + OpTrait::OpInvariants, AffineMapAccessInterface::Trait> { public: using Op::Op; static ArrayRef getAttributeNames() { return {}; } @@ -315,7 +316,8 @@ public: static StringRef getTagMapAttrName() { return "tag_map"; } static ParseResult parse(OpAsmParser &parser, OperationState &result); void print(OpAsmPrinter &p); - LogicalResult verifyInvariants(); + LogicalResult verifyInvariantsImpl(); + LogicalResult verifyInvariants() { return verifyInvariantsImpl(); } LogicalResult fold(ArrayRef cstOperands, SmallVectorImpl &results); }; diff --git a/mlir/include/mlir/IR/OpBase.td b/mlir/include/mlir/IR/OpBase.td index 5ad8bd4..11bc1a6 100644 --- a/mlir/include/mlir/IR/OpBase.td +++ b/mlir/include/mlir/IR/OpBase.td @@ -2023,6 +2023,10 @@ class PredAttrTrait : PredTrait; // OpTrait definitions //===----------------------------------------------------------------------===// +// A trait that describes the structure of operation will be marked with +// `StructuralOpTrait` and they will be verified first. +class StructuralOpTrait; + // These classes are used to define operation specific traits. class NativeOpTrait traits = []> : NativeTrait { @@ -2053,7 +2057,8 @@ class PredOpTrait traits = []> // Op defines an affine scope. def AffineScope : NativeOpTrait<"AffineScope">; // Op defines an automatic allocation scope. -def AutomaticAllocationScope : NativeOpTrait<"AutomaticAllocationScope">; +def AutomaticAllocationScope : + NativeOpTrait<"AutomaticAllocationScope">; // Op supports operand broadcast behavior. def ResultsBroadcastableShape : NativeOpTrait<"ResultsBroadcastableShape">; @@ -2074,9 +2079,11 @@ def SameTypeOperands : NativeOpTrait<"SameTypeOperands">; // Op has same shape for all operands. def SameOperandsShape : NativeOpTrait<"SameOperandsShape">; // Op has same operand and result shape. -def SameOperandsAndResultShape : NativeOpTrait<"SameOperandsAndResultShape">; +def SameOperandsAndResultShape : + NativeOpTrait<"SameOperandsAndResultShape">; // Op has the same element type (or type itself, if scalar) for all operands. -def SameOperandsElementType : NativeOpTrait<"SameOperandsElementType">; +def SameOperandsElementType : + NativeOpTrait<"SameOperandsElementType">; // Op has the same operand and result element type (or type itself, if scalar). def SameOperandsAndResultElementType : NativeOpTrait<"SameOperandsAndResultElementType">; @@ -2104,21 +2111,23 @@ def ElementwiseMappable : TraitList<[ ]>; // Op's regions have a single block. -def SingleBlock : NativeOpTrait<"SingleBlock">; +def SingleBlock : NativeOpTrait<"SingleBlock">, StructuralOpTrait; // Op's regions have a single block with the specified terminator. class SingleBlockImplicitTerminator - : ParamNativeOpTrait<"SingleBlockImplicitTerminator", op>; + : ParamNativeOpTrait<"SingleBlockImplicitTerminator", op>, + StructuralOpTrait; // Op's regions don't have terminator. -def NoTerminator : NativeOpTrait<"NoTerminator">; +def NoTerminator : NativeOpTrait<"NoTerminator">, StructuralOpTrait; // Op's parent operation is the provided one. class HasParent - : ParamNativeOpTrait<"HasParent", op>; + : ParamNativeOpTrait<"HasParent", op>, StructuralOpTrait; class ParentOneOf ops> - : ParamNativeOpTrait<"HasParent", !interleave(ops, ", ")>; + : ParamNativeOpTrait<"HasParent", !interleave(ops, ", ")>, + StructuralOpTrait; // Op result type is derived from the first attribute. If the attribute is an // subclass of `TypeAttrBase`, its value is used, otherwise, the type of the @@ -2147,13 +2156,15 @@ def SameVariadicResultSize : GenInternalOpTrait<"SameVariadicResultSize">; // vector that has the same number of elements as the number of ODS declared // operands. That means even if some operands are non-variadic, the attribute // still need to have an element for its size, which is always 1. -def AttrSizedOperandSegments : NativeOpTrait<"AttrSizedOperandSegments">; +def AttrSizedOperandSegments : + NativeOpTrait<"AttrSizedOperandSegments">, StructuralOpTrait; // Similar to AttrSizedOperandSegments, but used for results. The attribute // should be named as `result_segment_sizes`. -def AttrSizedResultSegments : NativeOpTrait<"AttrSizedResultSegments">; +def AttrSizedResultSegments : + NativeOpTrait<"AttrSizedResultSegments">, StructuralOpTrait; // Op attached regions have no arguments -def NoRegionArguments : NativeOpTrait<"NoRegionArguments">; +def NoRegionArguments : NativeOpTrait<"NoRegionArguments">, StructuralOpTrait; //===----------------------------------------------------------------------===// // OpInterface definitions @@ -2191,6 +2202,11 @@ class OpInterfaceTrait dependentTraits = traits; @@ -2467,6 +2483,16 @@ class Op props = []> { // operation class. The operation should implement this method and verify the // additional necessary invariants. bit hasVerifier = 0; + + // A bit indicating if the operation has additional invariants that need to + // verified and which associate with regions (aside from those verified by the + // traits). If set to `1`, an additional `LogicalResult verifyRegions()` + // declaration will be generated on the operation class. The operation should + // implement this method and verify the additional necessary invariants + // associated with regions. Note that this method is invoked after all the + // region ops are verified. + bit hasRegionVerifier = 0; + // A custom code block corresponding to the extra verification code of the // operation. // NOTE: This field is deprecated in favor of `hasVerifier` and is slated for diff --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h index e38b0cb..268b666 100644 --- a/mlir/include/mlir/IR/OpDefinition.h +++ b/mlir/include/mlir/IR/OpDefinition.h @@ -200,7 +200,8 @@ public: protected: /// If the concrete type didn't implement a custom verifier hook, just fall /// back to this one which accepts everything. - LogicalResult verifyInvariants() { return success(); } + LogicalResult verify() { return success(); } + LogicalResult verifyRegions() { return success(); } /// Parse the custom form of an operation. Unless overridden, this method will /// first try to get an operation parser from the op's dialect. Otherwise the @@ -376,6 +377,18 @@ struct MultiOperandTraitBase : public TraitBase { }; } // namespace detail +/// `verifyInvariantsImpl` verifies the invariants like the types, attrs, .etc. +/// It should be run after core traits and before any other user defined traits. +/// In order to run it in the correct order, wrap it with OpInvariants trait so +/// that tblgen will be able to put it in the right order. +template +class OpInvariants : public TraitBase { +public: + static LogicalResult verifyTrait(Operation *op) { + return cast(op).verifyInvariantsImpl(); + } +}; + /// This class provides the API for ops that are known to have no /// SSA operand. template @@ -1572,6 +1585,14 @@ using has_verify_trait = decltype(T::verifyTrait(std::declval())); template using detect_has_verify_trait = llvm::is_detected; +/// Trait to check if T provides a `verifyTrait` method. +template +using has_verify_region_trait = + decltype(T::verifyRegionTrait(std::declval())); +template +using detect_has_verify_region_trait = + llvm::is_detected; + /// The internal implementation of `verifyTraits` below that returns the result /// of verifying the current operation with all of the provided trait types /// `Ts`. @@ -1589,6 +1610,26 @@ template static LogicalResult verifyTraits(Operation *op) { return verifyTraitsImpl(op, (TraitTupleT *)nullptr); } + +/// The internal implementation of `verifyRegionTraits` below that returns the +/// result of verifying the current operation with all of the provided trait +/// types `Ts`. +template +static LogicalResult verifyRegionTraitsImpl(Operation *op, + std::tuple *) { + LogicalResult result = success(); + (void)std::initializer_list{ + (result = succeeded(result) ? Ts::verifyRegionTrait(op) : failure(), + 0)...}; + return result; +} + +/// Given a tuple type containing a set of traits that contain a +/// `verifyTrait` method, return the result of verifying the given operation. +template +static LogicalResult verifyRegionTraits(Operation *op) { + return verifyRegionTraitsImpl(op, (TraitTupleT *)nullptr); +} } // namespace op_definition_impl //===----------------------------------------------------------------------===// @@ -1603,7 +1644,8 @@ class Op : public OpState, public Traits... { public: /// Inherit getOperation from `OpState`. using OpState::getOperation; - using OpState::verifyInvariants; + using OpState::verify; + using OpState::verifyRegions; /// Return if this operation contains the provided trait. template