[Mlir-commits] [mlir] 4957518 - [mlir][ods] Simplify useDefaultType/AttributePrinterParser

llvmlistbot at llvm.org llvmlistbot at llvm.org
Wed May 18 10:22:15 PDT 2022


Author: Mogball
Date: 2022-05-18T17:22:11Z
New Revision: 4957518ef57f35e64d0b9716b745c654efa91d36

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

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

Added: 
    

Modified: 
    mlir/examples/toy/Ch7/include/toy/Ops.td
    mlir/include/mlir/Dialect/Async/IR/AsyncDialect.td
    mlir/include/mlir/Dialect/DLTI/DLTIBase.td
    mlir/include/mlir/Dialect/EmitC/IR/EmitCBase.td
    mlir/include/mlir/Dialect/GPU/GPUBase.td
    mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
    mlir/include/mlir/Dialect/Linalg/IR/LinalgBase.td
    mlir/include/mlir/Dialect/NVGPU/NVGPU.td
    mlir/include/mlir/Dialect/PDL/IR/PDLDialect.td
    mlir/include/mlir/Dialect/Quant/QuantOpsBase.td
    mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td
    mlir/include/mlir/Dialect/Shape/IR/ShapeBase.td
    mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
    mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorBase.td
    mlir/include/mlir/Dialect/Vector/IR/VectorOps.td
    mlir/include/mlir/IR/DialectBase.td
    mlir/test/python/python_test_ops.td
    mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
    mlir/tools/mlir-tblgen/DialectGen.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/examples/toy/Ch7/include/toy/Ops.td b/mlir/examples/toy/Ch7/include/toy/Ops.td
index cb59b368d0274..6b462c56c1b7b 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 4bf56586fa950..e14186600faba 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 8a1004df3b5cc..866cc934e05cb 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 01dc779674197..375dbcbce1d03 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 ecc457e4bb941..d19050bb3c653 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 4e540edb5e6f7..1b986968f928a 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 a1a8477b9bee4..8b9e039ccbd81 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 f8ed7237b5587..92e7a42cefa12 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 4c082c3518912..d405bec26634c 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 6431269557ac1..74d69c046f06b 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 616aeea03c6b8..45f07a9827b06 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 a32cbe1ec5574..1068e3219ca41 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 22a348230b04b..36f7c7c8d03e5 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 4aa2b89128c55..190ce3b751ded 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 1d3ac261fdfe5..152e5387c2e0a 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 8ce5d9558b0fd..df0a164d0f5de 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 958e2fce8740a..78ae1895558fa 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 64bccc5025de3..6e32b431bf823 100644
--- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
+++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
@@ -574,11 +574,9 @@ class DefGenerator {
 
 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 @@ 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.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 8944e83441871..c7e42ac0ac8fd 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;
 }
 


        


More information about the Mlir-commits mailing list