From 0d86e53e188490aa7ab8ff0f860dc12c48d7c2f4 Mon Sep 17 00:00:00 2001 From: River Riddle Date: Wed, 2 Feb 2022 10:24:43 -0800 Subject: [PATCH] [mlir][NFC] Update PDL operations to use `hasVerifier` instead of `verifier` The verifier field is deprecated, and slated for removal. Differential Revision: https://reviews.llvm.org/D118828 --- mlir/include/mlir/Dialect/PDL/IR/PDLOps.td | 15 ++- .../mlir/Dialect/PDLInterp/IR/PDLInterpOps.td | 21 +-- mlir/lib/Dialect/PDL/IR/PDL.cpp | 123 +++++++++--------- mlir/lib/Dialect/PDLInterp/IR/PDLInterp.cpp | 63 ++++++++- 4 files changed, 136 insertions(+), 86 deletions(-) diff --git a/mlir/include/mlir/Dialect/PDL/IR/PDLOps.td b/mlir/include/mlir/Dialect/PDL/IR/PDLOps.td index fe2ad1f2f16d..7f7b8a6a06d9 100644 --- a/mlir/include/mlir/Dialect/PDL/IR/PDLOps.td +++ b/mlir/include/mlir/Dialect/PDL/IR/PDLOps.td @@ -26,7 +26,6 @@ class PDL_Op traits = []> : Op { let printer = [{ ::print(p, *this); }]; let parser = [{ return ::parse$cppClass(parser, result); }]; - let verifier = [{ return ::verify(*this); }]; } //===----------------------------------------------------------------------===// @@ -66,6 +65,7 @@ def PDL_ApplyNativeConstraintOp params.empty() ? ArrayAttr() : $_builder.getArrayAttr(params)); }]>, ]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -118,6 +118,7 @@ def PDL_ApplyNativeRewriteOp $name ($constParams^)? (`(` $args^ `:` type($args) `)`)? (`:` type($results)^)? attr-dict }]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -164,6 +165,7 @@ def PDL_AttributeOp : PDL_Op<"attribute", [NoSideEffect]> { build($_builder, $_state, $_builder.getType(), Value(), attr); }]>, ]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -185,7 +187,6 @@ def PDL_EraseOp : PDL_Op<"erase", [HasParent<"pdl::RewriteOp">]> { }]; let arguments = (ins PDL_Operation:$operation); let assemblyFormat = "$operation attr-dict"; - let verifier = ?; } //===----------------------------------------------------------------------===// @@ -224,6 +225,7 @@ def PDL_OperandOp build($_builder, $_state, $_builder.getType(), Value()); }]>, ]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -263,6 +265,7 @@ def PDL_OperandsOp Value()); }]>, ]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -396,6 +399,7 @@ def PDL_OperationOp : PDL_Op<"operation", [AttrSizedOperandSegments]> { /// inference. bool hasTypeInference(); }]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -452,6 +456,7 @@ def PDL_PatternOp : PDL_Op<"pattern", [ /// Returns the rewrite operation of this pattern. RewriteOp getRewriter(); }]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -492,6 +497,7 @@ def PDL_ReplaceOp : PDL_Op<"replace", [ $operation `with` (`(` $replValues^ `:` type($replValues) `)`)? ($replOperation^)? attr-dict }]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -524,7 +530,6 @@ def PDL_ResultOp : PDL_Op<"result", [NoSideEffect]> { let arguments = (ins PDL_Operation:$parent, I32Attr:$index); let results = (outs PDL_Value:$val); let assemblyFormat = "$index `of` $parent attr-dict"; - let verifier = ?; } //===----------------------------------------------------------------------===// @@ -567,6 +572,7 @@ def PDL_ResultsOp : PDL_Op<"results", [NoSideEffect]> { ($index^)? `of` $parent custom(ref($index), type($val)) attr-dict }]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -629,6 +635,7 @@ def PDL_RewriteOp : PDL_Op<"rewrite", [ ($body^)? attr-dict-with-keyword }]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -657,6 +664,7 @@ def PDL_TypeOp : PDL_Op<"type", [NoSideEffect]> { let arguments = (ins OptionalAttr:$type); let results = (outs PDL_Type:$result); let assemblyFormat = "attr-dict (`:` $type^)?"; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -685,6 +693,7 @@ def PDL_TypesOp : PDL_Op<"types", [NoSideEffect]> { let arguments = (ins OptionalAttr:$types); let results = (outs PDL_RangeOf:$result); let assemblyFormat = "attr-dict (`:` $types^)?"; + let hasVerifier = 1; } #endif // MLIR_DIALECT_PDL_IR_PDLOPS diff --git a/mlir/include/mlir/Dialect/PDLInterp/IR/PDLInterpOps.td b/mlir/include/mlir/Dialect/PDLInterp/IR/PDLInterpOps.td index 9196ae62afd2..e3d83970f303 100644 --- a/mlir/include/mlir/Dialect/PDLInterp/IR/PDLInterpOps.td +++ b/mlir/include/mlir/Dialect/PDLInterp/IR/PDLInterpOps.td @@ -75,19 +75,6 @@ class PDLInterp_SwitchOp traits = []> : PDLInterp_Op { let successors = (successor AnySuccessor:$defaultDest, VariadicSuccessor:$cases); - - let verifier = [{ - // Verify that the number of case destinations matches the number of case - // values. - size_t numDests = cases().size(); - size_t numValues = caseValues().size(); - if (numDests != numValues) { - return emitOpError("expected number of cases to match the number of case " - "values, got ") - << numDests << " but expected " << numValues; - } - return success(); - }]; } //===----------------------------------------------------------------------===// @@ -638,7 +625,7 @@ def PDLInterp_ForEachOp }]; let parser = [{ return ::parseForEachOp(parser, result); }]; let printer = [{ return ::print(p, *this); }]; - let verifier = [{ return ::verify(*this); }]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -1078,6 +1065,7 @@ def PDLInterp_SwitchAttributeOp build($_builder, $_state, attribute, $_builder.getArrayAttr(caseValues), defaultDest, dests); }]>]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -1111,6 +1099,7 @@ def PDLInterp_SwitchOperandCountOp build($_builder, $_state, operation, $_builder.getI32VectorAttr(counts), defaultDest, dests); }]>]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -1148,6 +1137,7 @@ def PDLInterp_SwitchOperationNameOp defaultDest, dests); }]>, ]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -1181,6 +1171,7 @@ def PDLInterp_SwitchResultCountOp build($_builder, $_state, operation, $_builder.getI32VectorAttr(counts), defaultDest, dests); }]>]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -1218,6 +1209,7 @@ def PDLInterp_SwitchTypeOp : PDLInterp_SwitchOp<"switch_type", [NoSideEffect]> { let extraClassDeclaration = [{ auto getCaseTypes() { return caseValues().getAsValueRange(); } }]; + let hasVerifier = 1; } //===----------------------------------------------------------------------===// @@ -1259,6 +1251,7 @@ def PDLInterp_SwitchTypesOp : PDLInterp_SwitchOp<"switch_types", let extraClassDeclaration = [{ auto getCaseTypes() { return caseValues().getAsRange(); } }]; + let hasVerifier = 1; } #endif // MLIR_DIALECT_PDLINTERP_IR_PDLINTERPOPS diff --git a/mlir/lib/Dialect/PDL/IR/PDL.cpp b/mlir/lib/Dialect/PDL/IR/PDL.cpp index e0f6753743e1..7e655e124771 100644 --- a/mlir/lib/Dialect/PDL/IR/PDL.cpp +++ b/mlir/lib/Dialect/PDL/IR/PDL.cpp @@ -90,9 +90,9 @@ static void visit(Operation *op, DenseSet &visited) { // pdl::ApplyNativeConstraintOp //===----------------------------------------------------------------------===// -static LogicalResult verify(ApplyNativeConstraintOp op) { - if (op.getNumOperands() == 0) - return op.emitOpError("expected at least one argument"); +LogicalResult ApplyNativeConstraintOp::verify() { + if (getNumOperands() == 0) + return emitOpError("expected at least one argument"); return success(); } @@ -100,9 +100,9 @@ static LogicalResult verify(ApplyNativeConstraintOp op) { // pdl::ApplyNativeRewriteOp //===----------------------------------------------------------------------===// -static LogicalResult verify(ApplyNativeRewriteOp op) { - if (op.getNumOperands() == 0 && op.getNumResults() == 0) - return op.emitOpError("expected at least one argument or result"); +LogicalResult ApplyNativeRewriteOp::verify() { + if (getNumOperands() == 0 && getNumResults() == 0) + return emitOpError("expected at least one argument or result"); return success(); } @@ -110,18 +110,18 @@ static LogicalResult verify(ApplyNativeRewriteOp op) { // pdl::AttributeOp //===----------------------------------------------------------------------===// -static LogicalResult verify(AttributeOp op) { - Value attrType = op.type(); - Optional attrValue = op.value(); +LogicalResult AttributeOp::verify() { + Value attrType = type(); + Optional attrValue = value(); if (!attrValue) { - if (isa(op->getParentOp())) - return op.emitOpError("expected constant value when specified within a " - "`pdl.rewrite`"); - return verifyHasBindingUse(op); + if (isa((*this)->getParentOp())) + return emitOpError( + "expected constant value when specified within a `pdl.rewrite`"); + return verifyHasBindingUse(*this); } if (attrType) - return op.emitOpError("expected only one of [`type`, `value`] to be set"); + return emitOpError("expected only one of [`type`, `value`] to be set"); return success(); } @@ -129,13 +129,13 @@ static LogicalResult verify(AttributeOp op) { // pdl::OperandOp //===----------------------------------------------------------------------===// -static LogicalResult verify(OperandOp op) { return verifyHasBindingUse(op); } +LogicalResult OperandOp::verify() { return verifyHasBindingUse(*this); } //===----------------------------------------------------------------------===// // pdl::OperandsOp //===----------------------------------------------------------------------===// -static LogicalResult verify(OperandsOp op) { return verifyHasBindingUse(op); } +LogicalResult OperandsOp::verify() { return verifyHasBindingUse(*this); } //===----------------------------------------------------------------------===// // pdl::OperationOp @@ -230,15 +230,15 @@ static LogicalResult verifyResultTypesAreInferrable(OperationOp op, return success(); } -static LogicalResult verify(OperationOp op) { - bool isWithinRewrite = isa(op->getParentOp()); - if (isWithinRewrite && !op.name()) - return op.emitOpError("must have an operation name when nested within " - "a `pdl.rewrite`"); - ArrayAttr attributeNames = op.attributeNames(); - auto attributeValues = op.attributes(); +LogicalResult OperationOp::verify() { + bool isWithinRewrite = isa((*this)->getParentOp()); + if (isWithinRewrite && !name()) + return emitOpError("must have an operation name when nested within " + "a `pdl.rewrite`"); + ArrayAttr attributeNames = attributeNamesAttr(); + auto attributeValues = attributes(); if (attributeNames.size() != attributeValues.size()) { - return op.emitOpError() + return emitOpError() << "expected the same number of attribute values and attribute " "names, got " << attributeNames.size() << " names and " << attributeValues.size() @@ -247,12 +247,12 @@ static LogicalResult verify(OperationOp op) { // If the operation is within a rewrite body and doesn't have type inference, // ensure that the result types can be resolved. - if (isWithinRewrite && !op.hasTypeInference()) { - if (failed(verifyResultTypesAreInferrable(op, op.types()))) + if (isWithinRewrite && !hasTypeInference()) { + if (failed(verifyResultTypesAreInferrable(*this, types()))) return failure(); } - return verifyHasBindingUse(op); + return verifyHasBindingUse(*this); } bool OperationOp::hasTypeInference() { @@ -269,12 +269,12 @@ bool OperationOp::hasTypeInference() { // pdl::PatternOp //===----------------------------------------------------------------------===// -static LogicalResult verify(PatternOp pattern) { - Region &body = pattern.body(); +LogicalResult PatternOp::verify() { + Region &body = getBodyRegion(); Operation *term = body.front().getTerminator(); auto rewriteOp = dyn_cast(term); if (!rewriteOp) { - return pattern.emitOpError("expected body to terminate with `pdl.rewrite`") + return emitOpError("expected body to terminate with `pdl.rewrite`") .attachNote(term->getLoc()) .append("see terminator defined here"); } @@ -283,8 +283,7 @@ static LogicalResult verify(PatternOp pattern) { // dialect. WalkResult result = body.walk([&](Operation *op) -> WalkResult { if (!isa_and_nonnull(op->getDialect())) { - pattern - .emitOpError("expected only `pdl` operations within the pattern body") + emitOpError("expected only `pdl` operations within the pattern body") .attachNote(op->getLoc()) .append("see non-`pdl` operation defined here"); return WalkResult::interrupt(); @@ -296,8 +295,7 @@ static LogicalResult verify(PatternOp pattern) { // Check that there is at least one operation. if (body.front().getOps().empty()) - return pattern.emitOpError( - "the pattern must contain at least one `pdl.operation`"); + return emitOpError("the pattern must contain at least one `pdl.operation`"); // Determine if the operations within the pdl.pattern form a connected // component. This is determined by starting the search from the first @@ -333,8 +331,7 @@ static LogicalResult verify(PatternOp pattern) { first = false; } else if (!visited.count(&op)) { // For the subsequent operations, check if already visited. - return pattern - .emitOpError("the operations must form a connected component") + return emitOpError("the operations must form a connected component") .attachNote(op.getLoc()) .append("see a disconnected value / operation here"); } @@ -364,10 +361,10 @@ StringRef PatternOp::getDefaultDialect() { // pdl::ReplaceOp //===----------------------------------------------------------------------===// -static LogicalResult verify(ReplaceOp op) { - if (op.replOperation() && !op.replValues().empty()) - return op.emitOpError() << "expected no replacement values to be provided" - " when the replacement operation is present"; +LogicalResult ReplaceOp::verify() { + if (replOperation() && !replValues().empty()) + return emitOpError() << "expected no replacement values to be provided" + " when the replacement operation is present"; return success(); } @@ -392,11 +389,11 @@ static void printResultsValueType(OpAsmPrinter &p, ResultsOp op, p << " -> " << resultType; } -static LogicalResult verify(ResultsOp op) { - if (!op.index() && op.getType().isa()) { - return op.emitOpError() << "expected `pdl.range` result type when " - "no index is specified, but got: " - << op.getType(); +LogicalResult ResultsOp::verify() { + if (!index() && getType().isa()) { + return emitOpError() << "expected `pdl.range` result type when " + "no index is specified, but got: " + << getType(); } return success(); } @@ -405,13 +402,13 @@ static LogicalResult verify(ResultsOp op) { // pdl::RewriteOp //===----------------------------------------------------------------------===// -static LogicalResult verify(RewriteOp op) { - Region &rewriteRegion = op.body(); +LogicalResult RewriteOp::verify() { + Region &rewriteRegion = body(); // Handle the case where the rewrite is external. - if (op.name()) { + if (name()) { if (!rewriteRegion.empty()) { - return op.emitOpError() + return emitOpError() << "expected rewrite region to be empty when rewrite is external"; } return success(); @@ -419,18 +416,18 @@ static LogicalResult verify(RewriteOp op) { // Otherwise, check that the rewrite region only contains a single block. if (rewriteRegion.empty()) { - return op.emitOpError() << "expected rewrite region to be non-empty if " - "external name is not specified"; + return emitOpError() << "expected rewrite region to be non-empty if " + "external name is not specified"; } // Check that no additional arguments were provided. - if (!op.externalArgs().empty()) { - return op.emitOpError() << "expected no external arguments when the " - "rewrite is specified inline"; + if (!externalArgs().empty()) { + return emitOpError() << "expected no external arguments when the " + "rewrite is specified inline"; } - if (op.externalConstParams()) { - return op.emitOpError() << "expected no external constant parameters when " - "the rewrite is specified inline"; + if (externalConstParams()) { + return emitOpError() << "expected no external constant parameters when " + "the rewrite is specified inline"; } return success(); @@ -445,9 +442,9 @@ StringRef RewriteOp::getDefaultDialect() { // pdl::TypeOp //===----------------------------------------------------------------------===// -static LogicalResult verify(TypeOp op) { - if (!op.typeAttr()) - return verifyHasBindingUse(op); +LogicalResult TypeOp::verify() { + if (!typeAttr()) + return verifyHasBindingUse(*this); return success(); } @@ -455,9 +452,9 @@ static LogicalResult verify(TypeOp op) { // pdl::TypesOp //===----------------------------------------------------------------------===// -static LogicalResult verify(TypesOp op) { - if (!op.typesAttr()) - return verifyHasBindingUse(op); +LogicalResult TypesOp::verify() { + if (!typesAttr()) + return verifyHasBindingUse(*this); return success(); } diff --git a/mlir/lib/Dialect/PDLInterp/IR/PDLInterp.cpp b/mlir/lib/Dialect/PDLInterp/IR/PDLInterp.cpp index 20d6a376b47d..039e38c855da 100644 --- a/mlir/lib/Dialect/PDLInterp/IR/PDLInterp.cpp +++ b/mlir/lib/Dialect/PDLInterp/IR/PDLInterp.cpp @@ -27,6 +27,21 @@ void PDLInterpDialect::initialize() { >(); } +template +static LogicalResult verifySwitchOp(OpT op) { + // Verify that the number of case destinations matches the number of case + // values. + size_t numDests = op.cases().size(); + size_t numValues = op.caseValues().size(); + if (numDests != numValues) { + return op.emitOpError( + "expected number of cases to match the number of case " + "values, got ") + << numDests << " but expected " << numValues; + } + return success(); +} + //===----------------------------------------------------------------------===// // pdl_interp::CreateOperationOp //===----------------------------------------------------------------------===// @@ -131,17 +146,17 @@ static void print(OpAsmPrinter &p, ForEachOp op) { p.printSuccessor(op.successor()); } -static LogicalResult verify(ForEachOp op) { +LogicalResult ForEachOp::verify() { // Verify that the operation has exactly one argument. - if (op.region().getNumArguments() != 1) - return op.emitOpError("requires exactly one argument"); + if (region().getNumArguments() != 1) + return emitOpError("requires exactly one argument"); // Verify that the loop variable and the operand (value range) // have compatible types. - BlockArgument arg = op.getLoopVariable(); + BlockArgument arg = getLoopVariable(); Type rangeType = pdl::RangeType::get(arg.getType()); - if (rangeType != op.values().getType()) - return op.emitOpError("operand must be a range of loop variable type"); + if (rangeType != values().getType()) + return emitOpError("operand must be a range of loop variable type"); return success(); } @@ -156,6 +171,42 @@ static Type getGetValueTypeOpValueType(Type type) { return type.isa() ? pdl::RangeType::get(valueTy) : valueTy; } +//===----------------------------------------------------------------------===// +// pdl_interp::SwitchAttributeOp +//===----------------------------------------------------------------------===// + +LogicalResult SwitchAttributeOp::verify() { return verifySwitchOp(*this); } + +//===----------------------------------------------------------------------===// +// pdl_interp::SwitchOperandCountOp +//===----------------------------------------------------------------------===// + +LogicalResult SwitchOperandCountOp::verify() { return verifySwitchOp(*this); } + +//===----------------------------------------------------------------------===// +// pdl_interp::SwitchOperationNameOp +//===----------------------------------------------------------------------===// + +LogicalResult SwitchOperationNameOp::verify() { return verifySwitchOp(*this); } + +//===----------------------------------------------------------------------===// +// pdl_interp::SwitchResultCountOp +//===----------------------------------------------------------------------===// + +LogicalResult SwitchResultCountOp::verify() { return verifySwitchOp(*this); } + +//===----------------------------------------------------------------------===// +// pdl_interp::SwitchTypeOp +//===----------------------------------------------------------------------===// + +LogicalResult SwitchTypeOp::verify() { return verifySwitchOp(*this); } + +//===----------------------------------------------------------------------===// +// pdl_interp::SwitchTypesOp +//===----------------------------------------------------------------------===// + +LogicalResult SwitchTypesOp::verify() { return verifySwitchOp(*this); } + //===----------------------------------------------------------------------===// // TableGen Auto-Generated Op and Interface Definitions //===----------------------------------------------------------------------===//