From c8e047f5e14c05cace5a8926c0f07b6281d6359f Mon Sep 17 00:00:00 2001 From: Mehdi Amini Date: Tue, 18 Jan 2022 06:33:21 +0000 Subject: [PATCH] Enable useDefault{Type/Attribute}PrinterParser by default in ODS Dialect definition The majority of dialects reimplement the same boilerplate over and over, switching the default makes it for better discoverability and make it simpler to implement new dialects. Differential Revision: https://reviews.llvm.org/D117524 --- mlir/include/mlir/IR/BuiltinDialect.td | 3 ++- mlir/include/mlir/IR/OpBase.td | 4 +-- mlir/lib/Dialect/Async/IR/Async.cpp | 19 -------------- mlir/lib/Dialect/EmitC/IR/EmitC.cpp | 20 --------------- mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp | 25 ------------------- mlir/lib/Dialect/PDL/IR/PDLTypes.cpp | 9 ------- .../SparseTensor/IR/SparseTensorDialect.cpp | 19 -------------- mlir/test/lib/Dialect/Test/TestDialect.td | 5 +++- mlir/test/mlir-tblgen/attr-or-type-format.td | 1 + mlir/test/mlir-tblgen/typedefs.td | 1 - mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp | 25 ++++++++++++++----- mlir/tools/mlir-tblgen/DialectGen.cpp | 4 +-- 12 files changed, 30 insertions(+), 105 deletions(-) diff --git a/mlir/include/mlir/IR/BuiltinDialect.td b/mlir/include/mlir/IR/BuiltinDialect.td index 321c0528fdf8..a3d0f0cbfd9a 100644 --- a/mlir/include/mlir/IR/BuiltinDialect.td +++ b/mlir/include/mlir/IR/BuiltinDialect.td @@ -19,9 +19,10 @@ include "mlir/IR/OpBase.td" def Builtin_Dialect : Dialect { let summary = "A dialect containing the builtin Attributes, Operations, and Types"; - let name = "builtin"; let cppNamespace = "::mlir"; + let useDefaultAttributePrinterParser = 0; + let useDefaultTypePrinterParser = 0; let extraClassDeclaration = [{ private: // Register the builtin Attributes. diff --git a/mlir/include/mlir/IR/OpBase.td b/mlir/include/mlir/IR/OpBase.td index 98ce1e4fe337..8d33e5a1edaa 100644 --- a/mlir/include/mlir/IR/OpBase.td +++ b/mlir/include/mlir/IR/OpBase.td @@ -314,11 +314,11 @@ class Dialect { // If this dialect should use default generated attribute parser boilerplate: // it'll dispatch the parsing to every individual attributes directly. - bit useDefaultAttributePrinterParser = 0; + bit useDefaultAttributePrinterParser = 1; // If this dialect should use default generated type parser boilerplate: // it'll dispatch the parsing to every individual types directly. - bit useDefaultTypePrinterParser = 0; + bit useDefaultTypePrinterParser = 1; // If this dialect overrides the hook for canonicalization patterns. bit hasCanonicalizer = 0; diff --git a/mlir/lib/Dialect/Async/IR/Async.cpp b/mlir/lib/Dialect/Async/IR/Async.cpp index c33d023c8d12..a3b9da1fccbb 100644 --- a/mlir/lib/Dialect/Async/IR/Async.cpp +++ b/mlir/lib/Dialect/Async/IR/Async.cpp @@ -346,22 +346,3 @@ Type ValueType::parse(mlir::AsmParser &parser) { } return ValueType::get(ty); } - -/// Print a type registered to this dialect. -void AsyncDialect::printType(Type type, DialectAsmPrinter &os) const { - if (failed(generatedTypePrinter(type, os))) - llvm_unreachable("unexpected 'async' type kind"); -} - -/// Parse a type registered to this dialect. -Type AsyncDialect::parseType(DialectAsmParser &parser) const { - StringRef typeTag; - if (parser.parseKeyword(&typeTag)) - return Type(); - Type genType; - auto parseResult = generatedTypeParser(parser, typeTag, genType); - if (parseResult.hasValue()) - return genType; - parser.emitError(parser.getNameLoc(), "unknown async type: ") << typeTag; - return {}; -} diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp index 69feca96b84d..3260443e81fe 100644 --- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp +++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp @@ -180,26 +180,6 @@ Attribute emitc::OpaqueAttr::parse(AsmParser &parser, Type type) { return get(parser.getContext(), value); } -Attribute EmitCDialect::parseAttribute(DialectAsmParser &parser, - Type type) const { - llvm::SMLoc typeLoc = parser.getCurrentLocation(); - StringRef mnemonic; - if (parser.parseKeyword(&mnemonic)) - return Attribute(); - Attribute genAttr; - OptionalParseResult parseResult = - generatedAttributeParser(parser, mnemonic, type, genAttr); - if (parseResult.hasValue()) - return genAttr; - parser.emitError(typeLoc, "unknown attribute in EmitC dialect"); - return Attribute(); -} - -void EmitCDialect::printAttribute(Attribute attr, DialectAsmPrinter &os) const { - if (failed(generatedAttributePrinter(attr, os))) - llvm_unreachable("unexpected 'EmitC' attribute kind"); -} - void emitc::OpaqueAttr::print(AsmPrinter &printer) const { printer << "<\""; llvm::printEscapedString(getValue(), printer.getStream()); diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp index e5fee6b02b9d..4448c3ac7749 100644 --- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp +++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp @@ -2906,28 +2906,3 @@ Attribute LoopOptionsAttr::parse(AsmParser &parser, Type type) { llvm::sort(options, llvm::less_first()); return get(parser.getContext(), options); } - -Attribute LLVMDialect::parseAttribute(DialectAsmParser &parser, - Type type) const { - if (type) { - parser.emitError(parser.getNameLoc(), "unexpected type"); - return {}; - } - StringRef attrKind; - if (parser.parseKeyword(&attrKind)) - return {}; - { - Attribute attr; - auto parseResult = generatedAttributeParser(parser, attrKind, type, attr); - if (parseResult.hasValue()) - return attr; - } - parser.emitError(parser.getNameLoc(), "unknown attribute type: ") << attrKind; - return {}; -} - -void LLVMDialect::printAttribute(Attribute attr, DialectAsmPrinter &os) const { - if (succeeded(generatedAttributePrinter(attr, os))) - return; - llvm_unreachable("Unknown attribute type"); -} diff --git a/mlir/lib/Dialect/PDL/IR/PDLTypes.cpp b/mlir/lib/Dialect/PDL/IR/PDLTypes.cpp index 2d357fa039fe..c92251971d26 100644 --- a/mlir/lib/Dialect/PDL/IR/PDLTypes.cpp +++ b/mlir/lib/Dialect/PDL/IR/PDLTypes.cpp @@ -53,15 +53,6 @@ static Type parsePDLType(AsmParser &parser) { return Type(); } -Type PDLDialect::parseType(DialectAsmParser &parser) const { - return parsePDLType(parser); -} - -void PDLDialect::printType(Type type, DialectAsmPrinter &printer) const { - if (failed(generatedTypePrinter(type, printer))) - llvm_unreachable("unknown 'pdl' type"); -} - //===----------------------------------------------------------------------===// // PDL Types //===----------------------------------------------------------------------===// diff --git a/mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp b/mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp index 024d380454d3..8f44b5b1b9e2 100644 --- a/mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp +++ b/mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp @@ -346,22 +346,3 @@ void SparseTensorDialect::initialize() { #define GET_OP_CLASSES #include "mlir/Dialect/SparseTensor/IR/SparseTensorOps.cpp.inc" - -Attribute SparseTensorDialect::parseAttribute(DialectAsmParser &parser, - Type type) const { - StringRef attrTag; - if (failed(parser.parseKeyword(&attrTag))) - return Attribute(); - Attribute attr; - auto parseResult = generatedAttributeParser(parser, attrTag, type, attr); - if (parseResult.hasValue()) - return attr; - parser.emitError(parser.getNameLoc(), "unknown sparse tensor attribute"); - return Attribute(); -} - -void SparseTensorDialect::printAttribute(Attribute attr, - DialectAsmPrinter &printer) const { - if (succeeded(generatedAttributePrinter(attr, printer))) - return; -} diff --git a/mlir/test/lib/Dialect/Test/TestDialect.td b/mlir/test/lib/Dialect/Test/TestDialect.td index 756c7c10c92d..2ac446f04834 100644 --- a/mlir/test/lib/Dialect/Test/TestDialect.td +++ b/mlir/test/lib/Dialect/Test/TestDialect.td @@ -22,7 +22,7 @@ def Test_Dialect : Dialect { let hasRegionResultAttrVerify = 1; let hasOperationInterfaceFallback = 1; let hasNonDefaultDestructor = 1; - let useDefaultAttributePrinterParser = 1; + let useDefaultTypePrinterParser = 0; let dependentDialects = ["::mlir::DLTIDialect"]; let extraClassDeclaration = [{ @@ -36,6 +36,9 @@ def Test_Dialect : Dialect { ::mlir::OpAsmPrinter &printer)> getOperationPrinter(::mlir::Operation *op) const override; + ::mlir::Type parseType(::mlir::DialectAsmParser &parser) const override; + void printType(::mlir::Type type, + ::mlir::DialectAsmPrinter &printer) const override; private: // Storage for a custom fallback interface. void *fallbackEffectOpInterfaces; diff --git a/mlir/test/mlir-tblgen/attr-or-type-format.td b/mlir/test/mlir-tblgen/attr-or-type-format.td index a0588bfb237d..d354c35480a9 100644 --- a/mlir/test/mlir-tblgen/attr-or-type-format.td +++ b/mlir/test/mlir-tblgen/attr-or-type-format.td @@ -7,6 +7,7 @@ include "mlir/IR/OpBase.td" def Test_Dialect : Dialect { let name = "TestDialect"; let cppNamespace = "::test"; + let useDefaultTypePrinterParser = 0; } class TestAttr : AttrDef; diff --git a/mlir/test/mlir-tblgen/typedefs.td b/mlir/test/mlir-tblgen/typedefs.td index 0e4ee23a45e5..733ca5e9b52f 100644 --- a/mlir/test/mlir-tblgen/typedefs.td +++ b/mlir/test/mlir-tblgen/typedefs.td @@ -34,7 +34,6 @@ include "mlir/IR/OpBase.td" def Test_Dialect: Dialect { // DECL-NOT: TestDialect -// DEF-NOT: TestDialect let name = "TestDialect"; let cppNamespace = "::test"; } diff --git a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp index d90adbc47ef4..f3528880e5f9 100644 --- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp +++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp @@ -616,9 +616,11 @@ public: protected: DefGenerator(std::vector &&defs, raw_ostream &os, - StringRef defType, StringRef valueType, bool isAttrGenerator) + StringRef defType, StringRef valueType, bool isAttrGenerator, + bool needsDialectParserPrinter) : defRecords(std::move(defs)), os(os), defType(defType), - valueType(valueType), isAttrGenerator(isAttrGenerator) {} + valueType(valueType), isAttrGenerator(isAttrGenerator), + needsDialectParserPrinter(needsDialectParserPrinter) {} /// Emit the list of def type names. void emitTypeDefList(ArrayRef defs); @@ -637,19 +639,29 @@ protected: /// Flag indicating if this generator is for Attributes. False if the /// generator is for types. bool isAttrGenerator; + /// Track if we need to emit the printAttribute/parseAttribute + /// implementations. + bool needsDialectParserPrinter; }; /// A specialized generator for AttrDefs. struct AttrDefGenerator : public DefGenerator { AttrDefGenerator(const llvm::RecordKeeper &records, raw_ostream &os) : DefGenerator(records.getAllDerivedDefinitions("AttrDef"), os, "Attr", - "Attribute", /*isAttrGenerator=*/true) {} + "Attribute", + /*isAttrGenerator=*/true, + /*needsDialectParserPrinter=*/ + !records.getAllDerivedDefinitions("DialectAttr").empty()) { + } }; /// A specialized generator for TypeDefs. struct TypeDefGenerator : public DefGenerator { TypeDefGenerator(const llvm::RecordKeeper &records, raw_ostream &os) : DefGenerator(records.getAllDerivedDefinitions("TypeDef"), os, "Type", - "Type", /*isAttrGenerator=*/false) {} + "Type", /*isAttrGenerator=*/false, + /*needsDialectParserPrinter=*/ + !records.getAllDerivedDefinitions("DialectType").empty()) { + } }; } // namespace @@ -860,7 +872,7 @@ bool DefGenerator::emitDefs(StringRef selectedDialect) { Dialect firstDialect = defs.front().getDialect(); // Emit the default parser/printer for Attributes if the dialect asked for // it. - if (valueType == "Attribute" && + if (valueType == "Attribute" && needsDialectParserPrinter && firstDialect.useDefaultAttributePrinterParser()) { NamespaceEmitter nsEmitter(os, firstDialect); os << llvm::formatv(dialectDefaultAttrPrinterParserDispatch, @@ -868,7 +880,8 @@ bool DefGenerator::emitDefs(StringRef selectedDialect) { } // Emit the default parser/printer for Types if the dialect asked for it. - if (valueType == "Type" && firstDialect.useDefaultTypePrinterParser()) { + if (valueType == "Type" && needsDialectParserPrinter && + firstDialect.useDefaultTypePrinterParser()) { NamespaceEmitter nsEmitter(os, firstDialect); os << llvm::formatv(dialectDefaultTypePrinterParserDispatch, firstDialect.getCppClassName()); diff --git a/mlir/tools/mlir-tblgen/DialectGen.cpp b/mlir/tools/mlir-tblgen/DialectGen.cpp index 7da5a3c84681..6338cf19d1b9 100644 --- a/mlir/tools/mlir-tblgen/DialectGen.cpp +++ b/mlir/tools/mlir-tblgen/DialectGen.cpp @@ -210,9 +210,9 @@ emitDialectDecl(Dialect &dialect, // Check for any attributes/types registered to this dialect. If there are, // add the hooks for parsing/printing. - if (!dialectAttrs.empty() || dialect.useDefaultAttributePrinterParser()) + if (!dialectAttrs.empty() && dialect.useDefaultAttributePrinterParser()) os << attrParserDecl; - if (!dialectTypes.empty() || dialect.useDefaultTypePrinterParser()) + if (!dialectTypes.empty() && dialect.useDefaultTypePrinterParser()) os << typeParserDecl; // Add the decls for the various features of the dialect.