[mlir] Add verify method to adaptor

This allows verifying op-indepent attributes (e.g., attributes that do not require the op to have been created) before constructing an operation. These include checking whether required attributes are defined or constraints on attributes (such as I32 attribute). This is not perfect (e.g., if one had a disjunctive constraint where one part relied on the op and the other doesn't, then this would not try and extract the op independent from the op dependent).

The next step is to move these out to a trait that could be verified earlier than in the generated method. The first use case is for inferring the return type while constructing the op. At that point you don't have an Operation yet and that ends up in one having to duplicate the same checks, e.g., verify that attribute A is defined before querying A in shape function which requires that duplication. Instead this allows one to invoke a method to verify all the traits and, if this is checked first during verification, then all other traits could use attributes knowing they have been verified.

It is a little bit funny to have these on the adaptor, but I see the adaptor as a place to collect information about the op before the op is constructed (e.g., avoiding stringly typed accessors, verifying what is possible to verify before the op is constructed) while being cheap to use even with constructed op (so layer of indirection between the op constructed/being constructed). And from that point of view it made sense to me.

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

mlir/docs/OpDefinitions.md

@@ -626,7 +626,8 @@ let verifier = [{
 ```
 
 Code placed in `verifier` will be called after the auto-generated verification
-code.
+code. The order of trait verification excluding those of `verifier` should not
+be relied upon.
 
 ### Declarative Assembly Format
 

mlir/test/Dialect/GPU/invalid.mlir

@@ -254,7 +254,7 @@ func @reduce_op_and_body(%arg0 : f32) {
 // -----
 
 func @reduce_invalid_op(%arg0 : f32) {
-  // expected-error@+1 {{gpu.all_reduce' op attribute 'op' failed to satisfy constraint}}
+  // expected-error@+1 {{attribute 'op' failed to satisfy constraint}}
   %res = "gpu.all_reduce"(%arg0) ({}) {op = "foo"} : (f32) -> (f32)
   return
 }
@@ -321,14 +321,14 @@ func @reduce_incorrect_yield(%arg0 : f32) {
 // -----
 
 func @shuffle_mismatching_type(%arg0 : f32, %arg1 : i32, %arg2 : i32) {
-  // expected-error@+1 {{'gpu.shuffle' op requires the same type for value operand and result}}
+  // expected-error@+1 {{requires the same type for value operand and result}}
   %shfl, %pred = "gpu.shuffle"(%arg0, %arg1, %arg2) { mode = "xor" } : (f32, i32, i32) -> (i32, i1)
 }
 
 // -----
 
 func @shuffle_unsupported_type(%arg0 : index, %arg1 : i32, %arg2 : i32) {
-  // expected-error@+1 {{'gpu.shuffle' op requires value operand type to be f32 or i32}}
+  // expected-error@+1 {{requires value operand type to be f32 or i32}}
   %shfl, %pred = gpu.shuffle %arg0, %arg1, %arg2 xor : index
 }
 

mlir/test/Dialect/LLVMIR/global.mlir

@@ -65,12 +65,12 @@ func @references() {
 
 // -----
 
-// expected-error @+1 {{op requires string attribute 'sym_name'}}
+// expected-error @+1 {{requires string attribute 'sym_name'}}
 "llvm.mlir.global"() ({}) {type = !llvm.i64, constant, value = 42 : i64} : () -> ()
 
 // -----
 
-// expected-error @+1 {{op requires attribute 'type'}}
+// expected-error @+1 {{requires attribute 'type'}}
 "llvm.mlir.global"() ({}) {sym_name = "foo", constant, value = 42 : i64} : () -> ()
 
 // -----

mlir/test/Dialect/SPIRV/composite-ops.mlir

@@ -124,7 +124,7 @@ func @composite_extract_invalid_index_type_1() -> () {
 // -----
 
 func @composite_extract_invalid_index_type_2(%arg0 : !spv.array<4x!spv.array<4xf32>>) -> () {
-  // expected-error @+1 {{op attribute 'indices' failed to satisfy constraint: 32-bit integer array attribute}}
+  // expected-error @+1 {{attribute 'indices' failed to satisfy constraint: 32-bit integer array attribute}}
   %0 = spv.CompositeExtract %arg0[1] : !spv.array<4x!spv.array<4xf32>>
   return
 }

mlir/test/Dialect/Vector/invalid.mlir

@@ -1069,7 +1069,7 @@ func @reduce_elt_type_mismatch(%arg0: vector<16xf32>) -> i32 {
 // -----
 
 func @reduce_unsupported_attr(%arg0: vector<16xf32>) -> i32 {
-  // expected-error@+1 {{'vector.reduction' op attribute 'kind' failed to satisfy constraint: string attribute}}
+  // expected-error@+1 {{attribute 'kind' failed to satisfy constraint: string attribute}}
   %0 = vector.reduction 1234, %arg0 : vector<16xf32> into i32
 }
 

mlir/test/IR/invalid-ops.mlir

@@ -58,7 +58,7 @@ func @constant_wrong_type() {
 func @affine_apply_no_map() {
 ^bb0:
   %i = constant 0 : index
-  %x = "affine.apply" (%i) { } : (index) -> (index) //  expected-error {{'affine.apply' op requires attribute 'map'}}
+  %x = "affine.apply" (%i) { } : (index) -> (index) //  expected-error {{requires attribute 'map'}}
   return
 }
 
@@ -1205,7 +1205,7 @@ func @assume_alignment(%0: memref<4x4xf16>) {
 
 // 0 alignment value.
 func @assume_alignment(%0: memref<4x4xf16>) {
-  // expected-error@+1 {{'std.assume_alignment' op attribute 'alignment' failed to satisfy constraint: 32-bit signless integer attribute whose value is positive}}
+  // expected-error@+1 {{attribute 'alignment' failed to satisfy constraint: 32-bit signless integer attribute whose value is positive}}
   std.assume_alignment %0, 0 : memref<4x4xf16>
   return
 }

mlir/test/mlir-tblgen/op-attribute.td

@@ -30,6 +30,20 @@ def AOp : NS_Op<"a_op", []> {
 
 // DEF-LABEL: AOp definitions
 
+// Test verify method
+// ---
+
+// DEF:      LogicalResult AOpOperandAdaptor::verify
+// DEF:      auto tblgen_aAttr = odsAttrs.get("aAttr");
+// DEF-NEXT: if (!tblgen_aAttr) return emitError(loc, "'test.a_op' op ""requires attribute 'aAttr'");
+// DEF:        if (!((some-condition))) return emitError(loc, "'test.a_op' op ""attribute 'aAttr' failed to satisfy constraint: some attribute kind");
+// DEF:      auto tblgen_bAttr = odsAttrs.get("bAttr");
+// DEF-NEXT: if (tblgen_bAttr) {
+// DEF-NEXT:   if (!((some-condition))) return emitError(loc, "'test.a_op' op ""attribute 'bAttr' failed to satisfy constraint: some attribute kind");
+// DEF:      auto tblgen_cAttr = odsAttrs.get("cAttr");
+// DEF-NEXT: if (tblgen_cAttr) {
+// DEF-NEXT:   if (!((some-condition))) return emitError(loc, "'test.a_op' op ""attribute 'cAttr' failed to satisfy constraint: some attribute kind");
+
 // Test getter methods
 // ---
 
@@ -80,20 +94,6 @@ def AOp : NS_Op<"a_op", []> {
 // DEF:        ArrayRef<NamedAttribute> attributes
 // DEF:      odsState.addAttributes(attributes);
 
-// Test verify method
-// ---
-
-// DEF:      AOp::verify()
-// DEF:      auto tblgen_aAttr = this->getAttr("aAttr");
-// DEF-NEXT: if (!tblgen_aAttr) return emitOpError("requires attribute 'aAttr'");
-// DEF:        if (!((some-condition))) return emitOpError("attribute 'aAttr' failed to satisfy constraint: some attribute kind");
-// DEF:      auto tblgen_bAttr = this->getAttr("bAttr");
-// DEF-NEXT: if (tblgen_bAttr) {
-// DEF-NEXT:   if (!((some-condition))) return emitOpError("attribute 'bAttr' failed to satisfy constraint: some attribute kind");
-// DEF:      auto tblgen_cAttr = this->getAttr("cAttr");
-// DEF-NEXT: if (tblgen_cAttr) {
-// DEF-NEXT:   if (!((some-condition))) return emitOpError("attribute 'cAttr' failed to satisfy constraint: some attribute kind");
-
 def SomeTypeAttr : TypeAttrBase<"SomeType", "some type attribute">;
 
 def BOp : NS_Op<"b_op", []> {
@@ -114,27 +114,11 @@ def BOp : NS_Op<"b_op", []> {
   );
 }
 
-// Test common attribute kind getters' return types
-// ---
-
-// DEF: Attribute BOp::any_attr()
-// DEF: bool BOp::bool_attr()
-// DEF: APInt BOp::i32_attr()
-// DEF: APInt BOp::i64_attr()
-// DEF: APFloat BOp::f32_attr()
-// DEF: APFloat BOp::f64_attr()
-// DEF: StringRef BOp::str_attr()
-// DEF: ElementsAttr BOp::elements_attr()
-// DEF: StringRef BOp::function_attr()
-// DEF: SomeType BOp::type_attr()
-// DEF: ArrayAttr BOp::array_attr()
-// DEF: ArrayAttr BOp::some_attr_array()
-// DEF: Type BOp::type_attr()
 
 // Test common attribute kinds' constraints
 // ---
 
-// DEF-LABEL: BOp::verify
+// DEF-LABEL: BOpOperandAdaptor::verify
 // DEF: if (!((true)))
 // DEF: if (!((tblgen_bool_attr.isa<BoolAttr>())))
 // DEF: if (!(((tblgen_i32_attr.isa<IntegerAttr>())) && ((tblgen_i32_attr.cast<IntegerAttr>().getType().isSignlessInteger(32)))))
@@ -149,6 +133,23 @@ def BOp : NS_Op<"b_op", []> {
 // DEF: if (!(((tblgen_some_attr_array.isa<ArrayAttr>())) && (llvm::all_of(tblgen_some_attr_array.cast<ArrayAttr>(), [](Attribute attr) { return (some-condition); }))))
 // DEF: if (!(((tblgen_type_attr.isa<TypeAttr>())) && ((tblgen_type_attr.cast<TypeAttr>().getValue().isa<Type>()))))
 
+// Test common attribute kind getters' return types
+// ---
+
+// DEF: Attribute BOp::any_attr()
+// DEF: bool BOp::bool_attr()
+// DEF: APInt BOp::i32_attr()
+// DEF: APInt BOp::i64_attr()
+// DEF: APFloat BOp::f32_attr()
+// DEF: APFloat BOp::f64_attr()
+// DEF: StringRef BOp::str_attr()
+// DEF: ElementsAttr BOp::elements_attr()
+// DEF: StringRef BOp::function_attr()
+// DEF: SomeType BOp::type_attr()
+// DEF: ArrayAttr BOp::array_attr()
+// DEF: ArrayAttr BOp::some_attr_array()
+// DEF: Type BOp::type_attr()
+
 // Test building constant values for array attribute kinds
 // ---
 

mlir/test/mlir-tblgen/predicate.td

@@ -1,4 +1,4 @@
-// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s
+// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s --dump-input-on-failure
 
 include "mlir/IR/OpBase.td"
 
@@ -32,41 +32,41 @@ def OpF : NS_Op<"op_for_int_min_val", []> {
   let arguments = (ins Confined<I32Attr, [IntMinValue<10>]>:$attr);
 }
 
-// CHECK-LABEL: OpF::verify()
+// CHECK-LABEL: OpFOperandAdaptor::verify
 // CHECK:       (tblgen_attr.cast<IntegerAttr>().getInt() >= 10)
-// CHECK-SAME:    return emitOpError("attribute 'attr' failed to satisfy constraint: 32-bit signless integer attribute whose minimum value is 10");
+// CHECK-SAME:  "attribute 'attr' failed to satisfy constraint: 32-bit signless integer attribute whose minimum value is 10"
 
 def OpFX : NS_Op<"op_for_int_max_val", []> {
   let arguments = (ins Confined<I32Attr, [IntMaxValue<10>]>:$attr);
 }
 
-// CHECK-LABEL: OpFX::verify()
+// CHECK-LABEL: OpFXOperandAdaptor::verify
 // CHECK:       (tblgen_attr.cast<IntegerAttr>().getInt() <= 10)
-// CHECK-SAME:    return emitOpError("attribute 'attr' failed to satisfy constraint: 32-bit signless integer attribute whose maximum value is 10");
+// CHECK-SAME:  "attribute 'attr' failed to satisfy constraint: 32-bit signless integer attribute whose maximum value is 10"
 
 def OpG : NS_Op<"op_for_arr_min_count", []> {
   let arguments = (ins Confined<ArrayAttr, [ArrayMinCount<8>]>:$attr);
 }
 
-// CHECK-LABEL: OpG::verify()
+// CHECK-LABEL: OpGOperandAdaptor::verify
 // CHECK:       (tblgen_attr.cast<ArrayAttr>().size() >= 8)
-// CHECK-SAME:    return emitOpError("attribute 'attr' failed to satisfy constraint: array attribute with at least 8 elements");
+// CHECK-SAME:  "attribute 'attr' failed to satisfy constraint: array attribute with at least 8 elements"
 
 def OpH : NS_Op<"op_for_arr_value_at_index", []> {
   let arguments = (ins Confined<ArrayAttr, [IntArrayNthElemEq<0, 8>]>:$attr);
 }
 
-// CHECK-LABEL: OpH::verify()
+// CHECK-LABEL: OpHOperandAdaptor::verify
 // CHECK: (((tblgen_attr.cast<ArrayAttr>().size() > 0)) && ((tblgen_attr.cast<ArrayAttr>()[0].cast<IntegerAttr>().getInt() == 8)))))
-// CHECK-SAME:    return emitOpError("attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be 8");
+// CHECK-SAME:  "attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be 8"
 
 def OpI: NS_Op<"op_for_arr_min_value_at_index", []> {
   let arguments = (ins Confined<ArrayAttr, [IntArrayNthElemMinValue<0, 8>]>:$attr);
 }
 
-// CHECK-LABEL: OpI::verify()
+// CHECK-LABEL: OpIOperandAdaptor::verify
 // CHECK: (((tblgen_attr.cast<ArrayAttr>().size() > 0)) && ((tblgen_attr.cast<ArrayAttr>()[0].cast<IntegerAttr>().getInt() >= 8)))))
-// CHECK-SAME:    return emitOpError("attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be at least 8");
+// CHECK-SAME: "attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be at least 8"
 
 def OpJ: NS_Op<"op_for_TCopVTEtAreSameAt", [
                 PredOpTrait<"operands indexed at 0, 2, 3 should all have "
@@ -80,11 +80,11 @@ def OpJ: NS_Op<"op_for_TCopVTEtAreSameAt", [
   );
 }
 
-// CHECK-LABEL: OpJ::verify()
+// CHECK-LABEL: OpJOperandAdaptor::verify
 // CHECK:      llvm::is_splat(llvm::map_range(
 // CHECK-SAME:   llvm::ArrayRef<unsigned>({0, 2, 3}),
 // CHECK-SAME:   [this](unsigned i) { return getElementTypeOrSelf(this->getOperand(i)); }))
-// CHECK:   return emitOpError("failed to verify that operands indexed at 0, 2, 3 should all have the same type");
+// CHECK: "failed to verify that operands indexed at 0, 2, 3 should all have the same type"
 
 def OpK : NS_Op<"op_for_AnyTensorOf", []> {
   let arguments = (ins TensorOf<[F32, I32]>:$x);