[Mlir-commits] [mlir] c8e047f - Enable useDefault{Type/Attribute}PrinterParser by default in ODS Dialect definition

Mehdi Amini llvmlistbot at llvm.org
Mon Jan 17 22:38:09 PST 2022


Author: Mehdi Amini
Date: 2022-01-18T06:36:34Z
New Revision: c8e047f5e14c05cace5a8926c0f07b6281d6359f

URL: https://github.com/llvm/llvm-project/commit/c8e047f5e14c05cace5a8926c0f07b6281d6359f
DIFF: https://github.com/llvm/llvm-project/commit/c8e047f5e14c05cace5a8926c0f07b6281d6359f.diff

LOG: 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

Added: 
    

Modified: 
    mlir/include/mlir/IR/BuiltinDialect.td
    mlir/include/mlir/IR/OpBase.td
    mlir/lib/Dialect/Async/IR/Async.cpp
    mlir/lib/Dialect/EmitC/IR/EmitC.cpp
    mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
    mlir/lib/Dialect/PDL/IR/PDLTypes.cpp
    mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp
    mlir/test/lib/Dialect/Test/TestDialect.td
    mlir/test/mlir-tblgen/attr-or-type-format.td
    mlir/test/mlir-tblgen/typedefs.td
    mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
    mlir/tools/mlir-tblgen/DialectGen.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/BuiltinDialect.td b/mlir/include/mlir/IR/BuiltinDialect.td
index 321c0528fdf84..a3d0f0cbfd9a4 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 98ce1e4fe337f..8d33e5a1edaa9 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 c33d023c8d122..a3b9da1fccbb9 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 69feca96b84d2..3260443e81fed 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 e5fee6b02b9d0..4448c3ac77491 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 2d357fa039fee..c92251971d26e 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 024d380454d31..8f44b5b1b9e22 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 756c7c10c92db..2ac446f04834c 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 a0588bfb237df..d354c35480a95 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<string name> : AttrDef<Test_Dialect, name>;

diff  --git a/mlir/test/mlir-tblgen/typedefs.td b/mlir/test/mlir-tblgen/typedefs.td
index 0e4ee23a45e55..733ca5e9b52f0 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 d90adbc47ef4d..f3528880e5f9b 100644
--- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
+++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
@@ -616,9 +616,11 @@ class DefGenerator {
 
 protected:
   DefGenerator(std::vector<llvm::Record *> &&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<AttrOrTypeDef> defs);
@@ -637,19 +639,29 @@ class DefGenerator {
   /// 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 7da5a3c84681d..6338cf19d1b96 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.


        


More information about the Mlir-commits mailing list