[mlir][ods] Simplify useDefaultType/AttributePrinterParser

The current behaviour of `useDefaultTypePrinterParser` and `useDefaultAttributePrinterParser` is that they are set by default, but the dialect generator only generates the declarations for the parsing and printing hooks if it sees dialect types and attributes. Same goes for the definitions generated by the AttrOrTypeDef generator.

This can lead to confusing and undesirable behaviour if the dialect generator doesn't see the definitions of the attributes and types, for example, if they are sensibly separated into different files: `Dialect.td`, `Ops.td`, `Attributes.td`, and `Types.td`.

Now, these bits are unset by default. Setting them will always result in the dialect generator emitting the declarations for the parsing hooks. And if the AttrOrTypeDef generator sees it set, it will generate the default implementations.

Reviewed By: rriddle, stellaraccident

Differential Revision: https://reviews.llvm.org/D125809

19 files changed, 64 insertions, 51 deletions

diff --git a/mlir/examples/toy/Ch7/include/toy/Ops.td b/mlir/examples/toy/Ch7/include/toy/Ops.td
index cb59b368d027..6b462c56c1b7 100644
--- a/mlir/examples/toy/Ch7/include/toy/Ops.td
+++ b/mlir/examples/toy/Ch7/include/toy/Ops.td
@@ -30,6 +30,10 @@ def Toy_Dialect : Dialect {
   // We set this bit to generate a declaration of the `materializeConstant`
   // method so that we can materialize constants for our toy operations.
   let hasConstantMaterializer = 1;
+
+  // We set this bit to generate the declarations for the dialect's type parsing
+  // and printing hooks.
+  let useDefaultTypePrinterParser = 1;
 }
 
 // Base class for toy dialect operations. This operation inherits from the base
diff --git a/mlir/include/mlir/Dialect/Async/IR/AsyncDialect.td b/mlir/include/mlir/Dialect/Async/IR/AsyncDialect.td
index 4bf56586fa95..e14186600fab 100644
--- a/mlir/include/mlir/Dialect/Async/IR/AsyncDialect.td
+++ b/mlir/include/mlir/Dialect/Async/IR/AsyncDialect.td
@@ -21,20 +21,22 @@ include "mlir/IR/OpBase.td"
 
 def AsyncDialect : Dialect {
   let name = "async";
+  let cppNamespace = "::mlir::async";
 
   let summary = "Types and operations for async dialect";
   let description = [{
     This dialect contains operations for modeling asynchronous execution.
   }];
 
-  let cppNamespace = "::mlir::async";
+  let useDefaultTypePrinterParser = 1;
 
   let extraClassDeclaration = [{
-    // The name of a unit attribute on funcs that are allowed to have a blocking
-    // async.runtime.await ops. Only useful in combination with
-    // 'eliminate-blocking-await-ops' option, which in absence of this attribute
-    // might convert a func to a coroutine.
-    static constexpr StringRef kAllowedToBlockAttrName = "async.allowed_to_block";
+    /// The name of a unit attribute on funcs that are allowed to have a
+    /// blocking async.runtime.await ops. Only useful in combination with
+    /// 'eliminate-blocking-await-ops' option, which in absence of this
+    /// attribute might convert a func to a coroutine.
+    static constexpr StringRef kAllowedToBlockAttrName =
+        "async.allowed_to_block";
   }];
 
 }
diff --git a/mlir/include/mlir/Dialect/DLTI/DLTIBase.td b/mlir/include/mlir/Dialect/DLTI/DLTIBase.td
index 8a1004df3b5c..866cc934e05c 100644
--- a/mlir/include/mlir/Dialect/DLTI/DLTIBase.td
+++ b/mlir/include/mlir/Dialect/DLTI/DLTIBase.td
@@ -35,6 +35,8 @@ def DLTI_Dialect : Dialect {
     constexpr const static ::llvm::StringLiteral
     kDataLayoutEndiannessLittle = "little";
   }];
+
+  let useDefaultAttributePrinterParser = 1;
 }
 
 def DLTI_DataLayoutEntryAttr : DialectAttr<
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitCBase.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitCBase.td
index 01dc77967419..375dbcbce1d0 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitCBase.td
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitCBase.td
@@ -30,6 +30,7 @@ def EmitC_Dialect : Dialect {
 
   let hasConstantMaterializer = 1;
   let useDefaultTypePrinterParser = 1;
+  let useDefaultAttributePrinterParser = 1;
 }
 
 #endif // MLIR_DIALECT_EMITC_IR_EMITCBASE
diff --git a/mlir/include/mlir/Dialect/GPU/GPUBase.td b/mlir/include/mlir/Dialect/GPU/GPUBase.td
index ecc457e4bb94..d19050bb3c65 100644
--- a/mlir/include/mlir/Dialect/GPU/GPUBase.td
+++ b/mlir/include/mlir/Dialect/GPU/GPUBase.td
@@ -54,6 +54,7 @@ def GPU_Dialect : Dialect {
 
   let dependentDialects = ["arith::ArithmeticDialect"];
   let useDefaultAttributePrinterParser = 1;
+  let useDefaultTypePrinterParser = 1;
 }
 
 def GPU_AsyncToken : DialectType<
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
index 4e540edb5e6f..1b986968f928 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
@@ -26,9 +26,12 @@ def LLVM_Dialect : Dialect {
   let name = "llvm";
   let cppNamespace = "::mlir::LLVM";
 
+  let useDefaultTypePrinterParser = 1;
+  let useDefaultAttributePrinterParser = 1;
   let hasRegionArgAttrVerify = 1;
   let hasRegionResultAttrVerify = 1;
   let hasOperationAttrVerify = 1;
+
   let extraClassDeclaration = [{
     /// Name of the data layout attributes.
     static StringRef getDataLayoutAttrName() { return "llvm.data_layout"; }
diff --git a/mlir/include/mlir/Dialect/Linalg/IR/LinalgBase.td b/mlir/include/mlir/Dialect/Linalg/IR/LinalgBase.td
index a1a8477b9bee..8b9e039ccbd8 100644
--- a/mlir/include/mlir/Dialect/Linalg/IR/LinalgBase.td
+++ b/mlir/include/mlir/Dialect/Linalg/IR/LinalgBase.td
@@ -40,6 +40,7 @@ def Linalg_Dialect : Dialect {
     "memref::MemRefDialect",
     "tensor::TensorDialect",
   ];
+  let useDefaultAttributePrinterParser = 1;
   let hasCanonicalizer = 1;
   let hasOperationAttrVerify = 1;
   let hasConstantMaterializer = 1;
diff --git a/mlir/include/mlir/Dialect/NVGPU/NVGPU.td b/mlir/include/mlir/Dialect/NVGPU/NVGPU.td
index f8ed7237b558..92e7a42cefa1 100644
--- a/mlir/include/mlir/Dialect/NVGPU/NVGPU.td
+++ b/mlir/include/mlir/Dialect/NVGPU/NVGPU.td
@@ -32,7 +32,8 @@ def NVGPU_Dialect : Dialect {
     representing PTX specific operations while using MLIR high level concepts
     like memref and 2-D vector.
   }];
-  let useDefaultAttributePrinterParser = 1;
+
+  let useDefaultTypePrinterParser = 1;
 }
 
 /// Device-side synchronization token.
diff --git a/mlir/include/mlir/Dialect/PDL/IR/PDLDialect.td b/mlir/include/mlir/Dialect/PDL/IR/PDLDialect.td
index 4c082c351891..d405bec26634 100644
--- a/mlir/include/mlir/Dialect/PDL/IR/PDLDialect.td
+++ b/mlir/include/mlir/Dialect/PDL/IR/PDLDialect.td
@@ -20,8 +20,8 @@ include "mlir/IR/OpBase.td"
 //===----------------------------------------------------------------------===//
 
 def PDL_Dialect : Dialect {
-  string summary = "High level pattern definition dialect";
-  string description = [{
+  let summary = "High level pattern definition dialect";
+  let description = [{
     PDL presents a high level abstraction for the rewrite pattern infrastructure
     available in MLIR. This abstraction allows for representing patterns
     transforming MLIR, as MLIR. This allows for applying all of the benefits
@@ -64,6 +64,8 @@ def PDL_Dialect : Dialect {
 
   let name = "pdl";
   let cppNamespace = "::mlir::pdl";
+
+  let useDefaultTypePrinterParser = 1;
   let extraClassDeclaration = [{
     void registerTypes();
   }];
diff --git a/mlir/include/mlir/Dialect/Quant/QuantOpsBase.td b/mlir/include/mlir/Dialect/Quant/QuantOpsBase.td
index 6431269557ac..74d69c046f06 100644
--- a/mlir/include/mlir/Dialect/Quant/QuantOpsBase.td
+++ b/mlir/include/mlir/Dialect/Quant/QuantOpsBase.td
@@ -18,6 +18,8 @@ include "mlir/IR/OpBase.td"
 def Quantization_Dialect : Dialect {
   let name = "quant";
   let cppNamespace = "::mlir::quant";
+
+  let useDefaultTypePrinterParser = 1;
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td
index 616aeea03c6b..45f07a9827b0 100644
--- a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td
+++ b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td
@@ -47,6 +47,8 @@ def SPIRV_Dialect : Dialect {
   }];
 
   let cppNamespace = "::mlir::spirv";
+  let useDefaultTypePrinterParser = 1;
+  let useDefaultAttributePrinterParser = 1;
   let hasConstantMaterializer = 1;
   let hasOperationAttrVerify = 1;
   let hasRegionArgAttrVerify = 1;
diff --git a/mlir/include/mlir/Dialect/Shape/IR/ShapeBase.td b/mlir/include/mlir/Dialect/Shape/IR/ShapeBase.td
index a32cbe1ec557..1068e3219ca4 100644
--- a/mlir/include/mlir/Dialect/Shape/IR/ShapeBase.td
+++ b/mlir/include/mlir/Dialect/Shape/IR/ShapeBase.td
@@ -37,6 +37,7 @@ def ShapeDialect : Dialect {
   let cppNamespace = "::mlir::shape";
   let dependentDialects = ["arith::ArithmeticDialect", "tensor::TensorDialect"];
 
+  let useDefaultTypePrinterParser = 1;
   let hasConstantMaterializer = 1;
   let hasOperationAttrVerify = 1;
   let emitAccessorPrefix = kEmitAccessorPrefix_Prefixed;
diff --git a/mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td b/mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
index 22a348230b04..36f7c7c8d03e 100644
--- a/mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
+++ b/mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
@@ -1,4 +1,4 @@
-//===- Shape.td - Shape operations definition --------------*- tablegen -*-===//
+//===- ShapeOps.td - Shape operations definition -----------*- tablegen -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
diff --git a/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorBase.td b/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorBase.td
index 4aa2b89128c5..190ce3b751de 100644
--- a/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorBase.td
+++ b/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorBase.td
@@ -72,6 +72,8 @@ def SparseTensor_Dialect : Dialect {
     * [Kjolstad20] Fredrik Berg Kjolstad. Sparse Tensor Algebra Compilation.
     PhD thesis, MIT, February, 2020.
   }];
+
+  let useDefaultAttributePrinterParser = 1;
 }
 
 #endif // SPARSETENSOR_BASE
diff --git a/mlir/include/mlir/Dialect/Vector/IR/VectorOps.td b/mlir/include/mlir/Dialect/Vector/IR/VectorOps.td
index 1d3ac261fdfe..152e5387c2e0 100644
--- a/mlir/include/mlir/Dialect/Vector/IR/VectorOps.td
+++ b/mlir/include/mlir/Dialect/Vector/IR/VectorOps.td
@@ -23,6 +23,8 @@ include "mlir/Interfaces/ViewLikeInterface.td"
 def Vector_Dialect : Dialect {
   let name = "vector";
   let cppNamespace = "::mlir::vector";
+
+  let useDefaultAttributePrinterParser = 1;
   let hasConstantMaterializer = 1;
   let dependentDialects = ["arith::ArithmeticDialect"];
   let emitAccessorPrefix = kEmitAccessorPrefix_Prefixed;
diff --git a/mlir/include/mlir/IR/DialectBase.td b/mlir/include/mlir/IR/DialectBase.td
index 8ce5d9558b0f..df0a164d0f5d 100644
--- a/mlir/include/mlir/IR/DialectBase.td
+++ b/mlir/include/mlir/IR/DialectBase.td
@@ -73,13 +73,17 @@ class Dialect {
   // If this dialect overrides the hook for op interface fallback.
   bit hasOperationInterfaceFallback = 0;
 
-  // If this dialect should use default generated attribute parser boilerplate:
-  // it'll dispatch the parsing to every individual attributes directly.
-  bit useDefaultAttributePrinterParser = 1;
+  // If this dialect should use default generated attribute parser boilerplate.
+  // When set, ODS will generate declarations for the attribute parsing and
+  // printing hooks in the dialect and default implementations that dispatch to
+  // each individual attribute directly.
+  bit useDefaultAttributePrinterParser = 0;
 
   // If this dialect should use default generated type parser boilerplate:
-  // it'll dispatch the parsing to every individual types directly.
-  bit useDefaultTypePrinterParser = 1;
+  // When set, ODS will generate declarations for the type parsing and printing
+  // hooks in the dialect and default implementations that dispatch to each
+  // individual type directly.
+  bit useDefaultTypePrinterParser = 0;
 
   // If this dialect overrides the hook for canonicalization patterns.
   bit hasCanonicalizer = 0;
diff --git a/mlir/test/python/python_test_ops.td b/mlir/test/python/python_test_ops.td
index 958e2fce8740..78ae1895558f 100644
--- a/mlir/test/python/python_test_ops.td
+++ b/mlir/test/python/python_test_ops.td
@@ -17,6 +17,9 @@ include "mlir/Interfaces/InferTypeOpInterface.td"
 def Python_Test_Dialect : Dialect {
   let name = "python_test";
   let cppNamespace = "python_test";
+
+  let useDefaultTypePrinterParser = 1;
+  let useDefaultAttributePrinterParser = 1;
 }
 
 class TestType<string name, string typeMnemonic>
diff --git a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
index 64bccc5025de..6e32b431bf82 100644
--- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
+++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
@@ -574,11 +574,9 @@ public:
 
 protected:
   DefGenerator(std::vector<llvm::Record *> &&defs, raw_ostream &os,
-               StringRef defType, StringRef valueType, bool isAttrGenerator,
-               bool needsDialectParserPrinter)
+               StringRef defType, StringRef valueType, bool isAttrGenerator)
       : defRecords(std::move(defs)), os(os), defType(defType),
-        valueType(valueType), isAttrGenerator(isAttrGenerator),
-        needsDialectParserPrinter(needsDialectParserPrinter) {}
+        valueType(valueType), isAttrGenerator(isAttrGenerator) {}
 
   /// Emit the list of def type names.
   void emitTypeDefList(ArrayRef<AttrOrTypeDef> defs);
@@ -597,30 +595,19 @@ 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.getAllDerivedDefinitionsIfDefined("AttrDef"), os,
-                     "Attr", "Attribute",
-                     /*isAttrGenerator=*/true,
-                     /*needsDialectParserPrinter=*/
-                     !records.getAllDerivedDefinitions("DialectAttr").empty()) {
-  }
+                     "Attr", "Attribute", /*isAttrGenerator=*/true) {}
 };
 /// A specialized generator for TypeDefs.
 struct TypeDefGenerator : public DefGenerator {
   TypeDefGenerator(const llvm::RecordKeeper &records, raw_ostream &os)
       : DefGenerator(records.getAllDerivedDefinitionsIfDefined("TypeDef"), os,
-                     "Type", "Type",
-                     /*isAttrGenerator=*/false,
-                     /*needsDialectParserPrinter=*/
-                     !records.getAllDerivedDefinitions("DialectType").empty()) {
-  }
+                     "Type", "Type", /*isAttrGenerator=*/false) {}
 };
 } // namespace
 
@@ -879,10 +866,9 @@ 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" && needsDialectParserPrinter &&
-      firstDialect.useDefaultAttributePrinterParser()) {
+
+  // Emit the default parser/printer for Attributes if the dialect asked for it.
+  if (isAttrGenerator && firstDialect.useDefaultAttributePrinterParser()) {
     NamespaceEmitter nsEmitter(os, firstDialect);
     if (firstDialect.isExtensible()) {
       os << llvm::formatv(dialectDefaultAttrPrinterParserDispatch,
@@ -896,8 +882,7 @@ bool DefGenerator::emitDefs(StringRef selectedDialect) {
   }
 
   // Emit the default parser/printer for Types if the dialect asked for it.
-  if (valueType == "Type" && needsDialectParserPrinter &&
-      firstDialect.useDefaultTypePrinterParser()) {
+  if (!isAttrGenerator && firstDialect.useDefaultTypePrinterParser()) {
     NamespaceEmitter nsEmitter(os, firstDialect);
     if (firstDialect.isExtensible()) {
       os << llvm::formatv(dialectDefaultTypePrinterParserDispatch,
diff --git a/mlir/tools/mlir-tblgen/DialectGen.cpp b/mlir/tools/mlir-tblgen/DialectGen.cpp
index 8944e8344187..c7e42ac0ac8f 100644
--- a/mlir/tools/mlir-tblgen/DialectGen.cpp
+++ b/mlir/tools/mlir-tblgen/DialectGen.cpp
@@ -182,11 +182,7 @@ static const char *const operationInterfaceFallbackDecl = R"(
 )";
 
 /// Generate the declaration for the given dialect class.
-static void
-emitDialectDecl(Dialect &dialect,
-                const iterator_range<DialectFilterIterator> &dialectAttrs,
-                const iterator_range<DialectFilterIterator> &dialectTypes,
-                raw_ostream &os) {
+static void emitDialectDecl(Dialect &dialect, raw_ostream &os) {
   // Emit all nested namespaces.
   {
     NamespaceEmitter nsEmitter(os, dialect);
@@ -198,11 +194,13 @@ emitDialectDecl(Dialect &dialect,
     os << llvm::formatv(dialectDeclBeginStr, cppName, dialect.getName(),
                         superClassName);
 
-    // Check for any attributes/types registered to this dialect.  If there are,
-    // add the hooks for parsing/printing.
-    if (!dialectAttrs.empty() && dialect.useDefaultAttributePrinterParser())
+    // If the dialect requested the default attribute printer and parser, emit
+    // the declarations for the hooks.
+    if (dialect.useDefaultAttributePrinterParser())
       os << attrParserDecl;
-    if (!dialectTypes.empty() && dialect.useDefaultTypePrinterParser())
+    // If the dialect requested the default type printer and parser, emit the
+    // delcarations for the hooks.
+    if (dialect.useDefaultTypePrinterParser())
       os << typeParserDecl;
 
     // Add the decls for the various features of the dialect.
@@ -242,10 +240,7 @@ static bool emitDialectDecls(const llvm::RecordKeeper &recordKeeper,
   Optional<Dialect> dialect = findDialectToGenerate(dialects);
   if (!dialect)
     return true;
-  auto attrDefs = recordKeeper.getAllDerivedDefinitions("DialectAttr");
-  auto typeDefs = recordKeeper.getAllDerivedDefinitions("DialectType");
-  emitDialectDecl(*dialect, filterForDialect<Attribute>(attrDefs, *dialect),
-                  filterForDialect<Type>(typeDefs, *dialect), os);
+  emitDialectDecl(*dialect, os);
   return false;
 }