Enable useDefault{Type/Attribute}PrinterParser by default in ODS Dialect definition

[mehdi_amini]

Summary

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.

[rriddle]

mlir/lib/IR/BuiltinAttributes.cpp

14

Are these necessary?

mlir/lib/IR/Location.cpp

11

What is this used for?

[mehdi_amini]

Remove unnessary includes

[mehdi_amini]

mlir/lib/IR/Location.cpp

11

It was needed originally before I disabled it in the .td file for this dialect.

The generated printer/parser uses some AsmParser defined there.

[jpienaar]

switching the default makes it for better discoverability

So too would making it unset by default (?) and flagging a warning at build time asking it to be explicitly set either way and explaining benefit of having it on (so no warning if 0). That is not a behavior change but pushes people in a better direction. Or it could simply be used to give folks a heads-up before flipping. Like a PSA but in code that they can see in their presubmit.

The dialects changed here can be done in separate changes from the default flips.

[mehdi_amini]

committing the dialect changes without changing the default would require to change the setting on each dialect individually back and forth, so I'd rather avoid the unnecessary churn here.

[mehdi_amini]

Tweak the ODS generators logic to have the decl/def generators in sync in their logic to decide when to emit or not the print/parse Type/Attribute methods.

[rriddle]

mlir/lib/IR/BuiltinAttributes.cpp

14

Unresolved?

mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp

642 ↗

(On Diff #400711)

[mehdi_amini]

address comments

mlir/lib/IR/BuiltinAttributes.cpp

14

Strange, I thought I removed these...

mlir/include/mlir/IR/BuiltinDialect.td

 #ifndef BUILTIN_BASE
 #define BUILTIN_BASE
 
 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.
     void registerAttributes();
     // Register the builtin Location Attributes.
     void registerLocationAttributes();
     // Register the builtin Types.
     void registerTypes();
 
   public:
   }];
 }
 
 #endif // BUILTIN_BASE

mlir/include/mlir/IR/OpBase.td

class Dialect {
   // If this dialect overrides the hook for verifying region result attributes.
   bit hasRegionResultAttrVerify = 0;
 
   // 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 = 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;
 
   // Whether to emit raw/with no prefix or format changes, or emit with
   // accessor with prefix only and UpperCamel suffix or to emit accessors with
   // both.
   //

mlir/lib/Dialect/Async/IR/Async.cpp

 Type ValueType::parse(mlir::AsmParser &parser) {
   Type ty;
   if (parser.parseLess() || parser.parseType(ty) || parser.parseGreater()) {
     parser.emitError(parser.getNameLoc(), "failed to parse async value type");
     return Type();
   }
   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 {};
-}

mlir/lib/Dialect/EmitC/IR/EmitC.cpp

if (parser.parseOptionalString(&value)) {
     parser.emitError(loc) << "expected string";
     return Attribute();
   }
   if (parser.parseGreater())
     return Attribute();
   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());
   printer << "\">";
 }
 
 //===----------------------------------------------------------------------===//
 // EmitC Types

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

do {
     options.push_back(std::make_pair(*option, value));
   } while (succeeded(parser.parseOptionalComma()));
   if (failed(parser.parseGreater()))
     return {};
 
   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");
-}

mlir/lib/Dialect/PDL/IR/PDLTypes.cpp

static Type parsePDLType(AsmParser &parser) {
   // emits an error. We should rework the `generatedTypeParser` to better
   // support when the keyword is valid but the individual type parser itself
   // emits an error.
   parser.emitError(parser.getNameLoc(), "invalid 'pdl' type: `")
       << typeTag << "'";
   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
 //===----------------------------------------------------------------------===//
 
 bool PDLType::classof(Type type) {
   return llvm::isa<PDLDialect>(type.getDialect());
 }
 

mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp

#include "mlir/Dialect/SparseTensor/IR/SparseTensorAttrDefs.cpp.inc"
   addOperations<
 #define GET_OP_LIST
 #include "mlir/Dialect/SparseTensor/IR/SparseTensorOps.cpp.inc"
       >();
 }
 
 #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;
-}

mlir/lib/IR/BuiltinAttributes.cpp

 //===- BuiltinAttributes.cpp - MLIR Builtin Attribute Classes -------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "mlir/IR/BuiltinAttributes.h"
 #include "AttributeDetail.h"
 #include "mlir/IR/AffineMap.h"
 #include "mlir/IR/BuiltinDialect.h"
 #include "mlir/IR/Dialect.h"
+#include "mlir/IR/DialectImplementation.h"
 #include "mlir/IR/IntegerSet.h"
+#include "mlir/IR/OpImplementation.h"
 #include "mlir/IR/Operation.h"
 #include "mlir/IR/SymbolTable.h"
 #include "mlir/IR/Types.h"
 #include "mlir/Interfaces/DecodeAttributesInterfaces.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/TypeSwitch.h"
 #include "llvm/Support/Endian.h"
 
 using namespace mlir;
 using namespace mlir::detail;
 
 //===----------------------------------------------------------------------===//
 /// Tablegen Attribute Definitions
 //===----------------------------------------------------------------------===//

mlir/lib/IR/Location.cpp

 //===- Location.cpp - MLIR Location Classes -------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "mlir/IR/Location.h"
 #include "mlir/IR/BuiltinDialect.h"
+#include "mlir/IR/OpDefinition.h"
 #include "mlir/IR/Visitors.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/TypeSwitch.h"
 
 using namespace mlir;
 using namespace mlir::detail;
 
 //===----------------------------------------------------------------------===//

mlir/test/lib/Dialect/Test/TestDialect.td

def Test_Dialect : Dialect {
   let emitAccessorPrefix = kEmitAccessorPrefix_Prefixed;
   let hasCanonicalizer = 1;
   let hasConstantMaterializer = 1;
   let hasOperationAttrVerify = 1;
   let hasRegionArgAttrVerify = 1;
   let hasRegionResultAttrVerify = 1;
   let hasOperationInterfaceFallback = 1;
   let hasNonDefaultDestructor = 1;
-  let useDefaultAttributePrinterParser = 1;
+  let useDefaultTypePrinterParser = 0;
   let dependentDialects = ["::mlir::DLTIDialect"];
 
   let extraClassDeclaration = [{
     void registerAttributes();
     void registerTypes();
 
     // Provides a custom printing/parsing for some operations.
     ::llvm::Optional<ParseOpHook>

mlir/test/mlir-tblgen/attr-or-type-format.td

 // RUN: mlir-tblgen -gen-attrdef-defs -I %S/../../include %s | FileCheck %s --check-prefix=ATTR
 // RUN: mlir-tblgen -gen-typedef-defs -I %S/../../include %s | FileCheck %s --check-prefix=TYPE
 
 include "mlir/IR/OpBase.td"
 
 /// Test that attribute and type printers and parsers are correctly generated.
 def Test_Dialect : Dialect {
   let name = "TestDialect";
   let cppNamespace = "::test";
+  let useDefaultTypePrinterParser = 0;
 }
 
 class TestAttr<string name> : AttrDef<Test_Dialect, name>;
 class TestType<string name> : TypeDef<Test_Dialect, name>;
 
 def AttrParamA : AttrParameter<"TestParamA", "an attribute param A"> {
   let parser = "::parseAttrParamA($_parser, $_type)";
   let printer = "::printAttrParamA($_printer, $_self)";

mlir/tools/mlir-tblgen/DialectGen.cpp

// Emit all nested namespaces.
 
     // Emit the start of the decl.
     std::string cppName = dialect.getCppClassName();
     os << llvm::formatv(dialectDeclBeginStr, cppName, dialect.getName(),
                         dependentDialectRegistrations);
 
     // 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())
       os << attrParserDecl;
-    if (!dialectTypes.empty() || dialect.useDefaultTypePrinterParser())
+    if (!dialectTypes.empty())
       os << typeParserDecl;
 
     // Add the decls for the various features of the dialect.
     if (dialect.hasCanonicalizer())
       os << canonicalizerDecl;
     if (dialect.hasConstantMaterializer())
       os << constantMaterializerDecl;
     if (dialect.hasOperationAttrVerify())