From bbfec2a1b013688c6507aba13b1ddbffcb619665 Mon Sep 17 00:00:00 2001 From: River Riddle Date: Mon, 7 Mar 2022 01:33:58 -0800 Subject: [PATCH] [mlir] Remove the deprecated ODS Op verifier/parser/printer code blocks These have been deprecated for ~1 month now and can be removed. Differential Revision: https://reviews.llvm.org/D121090 --- flang/include/flang/Optimizer/Dialect/FIROps.td | 8 ++-- mlir/include/mlir/Dialect/GPU/GPUOps.td | 2 +- mlir/include/mlir/IR/OpBase.td | 12 ----- mlir/lib/Dialect/GPU/IR/GPUDialect.cpp | 17 +++---- mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp | 63 +++---------------------- 5 files changed, 20 insertions(+), 82 deletions(-) diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td index 646ced1..b1cc285 100644 --- a/flang/include/flang/Optimizer/Dialect/FIROps.td +++ b/flang/include/flang/Optimizer/Dialect/FIROps.td @@ -417,11 +417,9 @@ def fir_UndefOp : fir_OneResultOp<"undefined", [NoSideEffect]> { let results = (outs AnyType:$intype); let assemblyFormat = "type($intype) attr-dict"; - - let verifier = [{ - // allow `undef : ref` since it is a possible from transformations - return mlir::success(); - }]; + + // Note: we allow `undef : ref` since it is a possible from transformations. + let hasVerifier = 0; } def fir_ZeroOp : fir_OneResultOp<"zero_bits", [NoSideEffect]> { diff --git a/mlir/include/mlir/Dialect/GPU/GPUOps.td b/mlir/include/mlir/Dialect/GPU/GPUOps.td index 91e5185..2976ee3 100644 --- a/mlir/include/mlir/Dialect/GPU/GPUOps.td +++ b/mlir/include/mlir/Dialect/GPU/GPUOps.td @@ -901,7 +901,7 @@ def GPU_AllocOp : GPU_Op<"alloc", [ `(` $dynamicSizes `)` (`` `[` $symbolOperands^ `]`)? attr-dict `:` type($memref) }]; - let verifier = [{ return ::verify(*this); }]; + let hasVerifier = 1; let hasCanonicalizer = 1; } diff --git a/mlir/include/mlir/IR/OpBase.td b/mlir/include/mlir/IR/OpBase.td index 74fd7d6..73c301a 100644 --- a/mlir/include/mlir/IR/OpBase.td +++ b/mlir/include/mlir/IR/OpBase.td @@ -2446,12 +2446,6 @@ class Op props = []> { // provided. bit skipDefaultBuilders = 0; - // Custom parser and printer. - // NOTE: These fields are deprecated in favor of `assemblyFormat` or - // `hasCustomAssemblyFormat`, and are slated for deletion. - code parser = ?; - code printer = ?; - // Custom assembly format. /// This field corresponds to a declarative description of the assembly format /// for this operation. If populated, the `hasCustomAssemblyFormat` field is @@ -2483,12 +2477,6 @@ class Op props = []> { // 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 - // deletion. - code verifier = ?; - // Whether this op has associated canonicalization patterns. bit hasCanonicalizer = 0; diff --git a/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp b/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp index a8ff898..65ce9b0 100644 --- a/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp +++ b/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp @@ -1174,20 +1174,21 @@ LogicalResult MemsetOp::fold(ArrayRef operands, // GPU_AllocOp //===----------------------------------------------------------------------===// -static LogicalResult verify(AllocOp op) { - auto memRefType = op.memref().getType().cast(); +LogicalResult AllocOp::verify() { + auto memRefType = memref().getType().cast(); - if (static_cast(op.dynamicSizes().size()) != + if (static_cast(dynamicSizes().size()) != memRefType.getNumDynamicDims()) - return op.emitOpError("dimension operand count does not equal memref " - "dynamic dimension count"); + return emitOpError("dimension operand count does not equal memref " + "dynamic dimension count"); unsigned numSymbols = 0; if (!memRefType.getLayout().isIdentity()) numSymbols = memRefType.getLayout().getAffineMap().getNumSymbols(); - if (op.symbolOperands().size() != numSymbols) - return op.emitOpError("symbol operand count does not equal memref symbol " - "count"); + if (symbolOperands().size() != numSymbols) { + return emitOpError( + "symbol operand count does not equal memref symbol count"); + } return success(); } diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp index b7f7636..dd09f9e 100644 --- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp +++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp @@ -2136,60 +2136,28 @@ void OpEmitter::genParser() { if (hasStringAttribute(def, "assemblyFormat")) return; - bool hasCppFormat = def.getValueAsBit("hasCustomAssemblyFormat"); - if (!hasStringAttribute(def, "parser") && !hasCppFormat) + if (!def.getValueAsBit("hasCustomAssemblyFormat")) return; SmallVector paramList; paramList.emplace_back("::mlir::OpAsmParser &", "parser"); paramList.emplace_back("::mlir::OperationState &", "result"); - // If this uses the cpp format, only generate a declaration. - if (hasCppFormat) { - auto *method = opClass.declareStaticMethod("::mlir::ParseResult", "parse", - std::move(paramList)); - ERROR_IF_PRUNED(method, "parse", op); - return; - } - - PrintNote(op.getLoc(), - "`parser` and `printer` fields are deprecated and will be removed, " - "please use the `hasCustomAssemblyFormat` field instead"); - - auto *method = opClass.addStaticMethod("::mlir::ParseResult", "parse", - std::move(paramList)); + auto *method = opClass.declareStaticMethod("::mlir::ParseResult", "parse", + std::move(paramList)); ERROR_IF_PRUNED(method, "parse", op); - - FmtContext fctx; - fctx.addSubst("cppClass", opClass.getClassName()); - auto parser = def.getValueAsString("parser").ltrim().rtrim(" \t\v\f\r"); - method->body() << " " << tgfmt(parser, &fctx); } void OpEmitter::genPrinter() { if (hasStringAttribute(def, "assemblyFormat")) return; - // If this uses the cpp format, only generate a declaration. - if (def.getValueAsBit("hasCustomAssemblyFormat")) { - auto *method = opClass.declareMethod( - "void", "print", MethodParameter("::mlir::OpAsmPrinter &", "p")); - ERROR_IF_PRUNED(method, "print", op); + // Check to see if this op uses a c++ format. + if (!def.getValueAsBit("hasCustomAssemblyFormat")) return; - } - - auto *valueInit = def.getValueInit("printer"); - StringInit *stringInit = dyn_cast(valueInit); - if (!stringInit) - return; - - auto *method = opClass.addMethod( + auto *method = opClass.declareMethod( "void", "print", MethodParameter("::mlir::OpAsmPrinter &", "p")); ERROR_IF_PRUNED(method, "print", op); - FmtContext fctx; - fctx.addSubst("cppClass", opClass.getClassName()); - auto printer = stringInit->getValue().ltrim().rtrim(" \t\v\f\r"); - method->body() << " " << tgfmt(printer, &fctx); } /// Generate verification on native traits requiring attributes. @@ -2272,14 +2240,10 @@ void OpEmitter::genVerifier() { // This may not act as their expectation because this doesn't call any // verifiers of native/interface traits. Needs to review those use cases and // see if we should use the mlir::verify() instead. - auto *valueInit = def.getValueInit("verifier"); - StringInit *stringInit = dyn_cast(valueInit); - bool hasCustomVerifyCodeBlock = stringInit && !stringInit->getValue().empty(); - auto *method = opClass.addMethod("::mlir::LogicalResult", "verifyInvariants"); ERROR_IF_PRUNED(method, "verifyInvariants", op); auto &body = method->body(); - if (hasCustomVerifyCodeBlock || def.getValueAsBit("hasVerifier")) { + if (def.getValueAsBit("hasVerifier")) { body << " if(::mlir::succeeded(verifyInvariantsImpl()) && " "::mlir::succeeded(verify()))\n"; body << " return ::mlir::success();\n"; @@ -2290,22 +2254,9 @@ void OpEmitter::genVerifier() { } void OpEmitter::genCustomVerifier() { - auto *valueInit = def.getValueInit("verifier"); - StringInit *stringInit = dyn_cast(valueInit); - bool hasCustomVerifyCodeBlock = stringInit && !stringInit->getValue().empty(); - if (def.getValueAsBit("hasVerifier")) { auto *method = opClass.declareMethod("::mlir::LogicalResult", "verify"); ERROR_IF_PRUNED(method, "verify", op); - } else if (hasCustomVerifyCodeBlock) { - auto *method = opClass.addMethod("::mlir::LogicalResult", "verify"); - ERROR_IF_PRUNED(method, "verify", op); - auto &body = method->body(); - - FmtContext fctx; - fctx.addSubst("cppClass", opClass.getClassName()); - auto printer = stringInit->getValue().ltrim().rtrim(" \t\v\f\r"); - body << " " << tgfmt(printer, &fctx); } if (def.getValueAsBit("hasRegionVerifier")) { -- 2.7.4