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 321c052..a3d0f0c 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 98ce1e4..8d33e5a 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 c33d023..a3b9da1 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 69feca9..3260443 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 e5fee6b..4448c3a 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 2d357fa..c922519 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 024d380..8f44b5b 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 756c7c1..2ac446f 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 a0588bf..d354c35 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 0e4ee23..733ca5e 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 d90adbc..f352888 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 7da5a3c..6338cf1 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. -- 2.7.4