[mlir][ods] Ignore AttributeSelfTypeParameter in assembly formats

The attribute self type parameter is currently treated like any other attribute parameter in the assembly format. The self type parameter should be handled by the operation parser and printer and play no role in the generated parsers and printers of attributes.

Reviewed By: rriddle

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

mlir/lib/IR/AsmPrinter.cpp

@@ -1758,11 +1758,10 @@ void AsmPrinter::Impl::printAttribute(Attribute attr,
   if (succeeded(printAlias(attr)))
     return;
 
-  if (!isa<BuiltinDialect>(attr.getDialect()))
-    return printDialectAttribute(attr);
-
   auto attrType = attr.getType();
-  if (auto opaqueAttr = attr.dyn_cast<OpaqueAttr>()) {
+  if (!isa<BuiltinDialect>(attr.getDialect())) {
+    printDialectAttribute(attr);
+  } else if (auto opaqueAttr = attr.dyn_cast<OpaqueAttr>()) {
     printDialectSymbol(os, "#", opaqueAttr.getDialectNamespace(),
                        opaqueAttr.getAttrData());
   } else if (attr.isa<UnitAttr>()) {

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

@@ -55,7 +55,7 @@ def CompoundAttrNested : Test_Attr<"CompoundAttrNested"> {
 def AttrWithSelfTypeParam : Test_Attr<"AttrWithSelfTypeParam"> {
   let mnemonic = "attr_with_self_type_param";
   let parameters = (ins AttributeSelfTypeParameter<"">:$type);
-  let hasCustomAssemblyFormat = 1;
+  let assemblyFormat = "";
 }
 
 // An attribute testing AttributeSelfTypeParameter.
@@ -205,4 +205,13 @@ def TestAttrWithTypeParam : Test_Attr<"TestAttrWithTypeParam"> {
   let assemblyFormat = "`<` $int_type `,` $any_type `>`";
 }
 
+// Test self type parameter with assembly format.
+def TestAttrSelfTypeParameterFormat
+    : Test_Attr<"TestAttrSelfTypeParameterFormat"> {
+  let parameters = (ins "int":$a, AttributeSelfTypeParameter<"">:$type);
+
+  let mnemonic = "attr_self_type_format";
+  let assemblyFormat = "`<` $a `>`";
+}
+
 #endif // TEST_ATTRDEFS

mlir/test/lib/Dialect/Test/TestAttributes.cpp

@@ -27,21 +27,6 @@
 using namespace mlir;
 using namespace test;
 
-//===----------------------------------------------------------------------===//
-// AttrWithSelfTypeParamAttr
-//===----------------------------------------------------------------------===//
-
-Attribute AttrWithSelfTypeParamAttr::parse(AsmParser &parser, Type type) {
-  Type selfType;
-  if (parser.parseType(selfType))
-    return Attribute();
-  return get(parser.getContext(), selfType);
-}
-
-void AttrWithSelfTypeParamAttr::print(AsmPrinter &printer) const {
-  printer << " " << getType();
-}
-
 //===----------------------------------------------------------------------===//
 // AttrWithTypeBuilderAttr
 //===----------------------------------------------------------------------===//

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

@@ -14,7 +14,9 @@ attributes {
   // CHECK: #test.attr_params<42, 24>
   attr3 = #test.attr_params<42, 24>,
   // CHECK: #test.attr_with_type<i32, vector<4xi32>>
-  attr4 = #test.attr_with_type<i32, vector<4xi32>>
+  attr4 = #test.attr_with_type<i32, vector<4xi32>>,
+  // CHECK: #test.attr_self_type_format<5> : i32
+  attr5 = #test.attr_self_type_format<5> : i32
 }
 
 // CHECK-LABEL: @test_roundtrip_default_parsers_struct

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

@@ -162,6 +162,17 @@ def AttrC : TestAttr<"TestF"> {
   let assemblyFormat = "params";
 }
 
+/// Test attribute with self type parameter
+
+// ATTR: TestGAttr::parse
+// ATTR:   return TestGAttr::get
+// ATTR:     odsType
+def AttrD : TestAttr<"TestG"> {
+  let parameters = (ins "int":$a, AttributeSelfTypeParameter<"">:$type);
+  let mnemonic = "attr_d";
+  let assemblyFormat = "$a";
+}
+
 /// Test type parser and printer that mix variables and struct are generated
 /// correctly.
 

mlir/test/mlir-tblgen/testdialect-attrdefs.mlir

@@ -4,10 +4,10 @@
 // CHECK-SAME: #test.cmpnd_a<1, !test.smpla, [5, 6]>
 func.func private @compoundA() attributes {foo = #test.cmpnd_a<1, !test.smpla, [5, 6]>}
 
-// CHECK: test.result_has_same_type_as_attr #test<"attr_with_self_type_param i32"> -> i32
-%a = test.result_has_same_type_as_attr #test<"attr_with_self_type_param i32"> -> i32
+// CHECK: test.result_has_same_type_as_attr #test.attr_with_self_type_param : i32 -> i32
+%a = test.result_has_same_type_as_attr #test.attr_with_self_type_param : i32 -> i32
 
-// CHECK: test.result_has_same_type_as_attr #test<"attr_with_type_builder 10 : i16"> -> i16
+// CHECK: test.result_has_same_type_as_attr #test<"attr_with_type_builder 10 : i16"> : i16 -> i16
 %b = test.result_has_same_type_as_attr #test<"attr_with_type_builder 10 : i16"> -> i16
 
 // CHECK-LABEL: @qualifiedAttr()

mlir/tools/mlir-tblgen/AttrOrTypeFormatGen.cpp

@@ -260,6 +260,8 @@ void DefFormat::genParser(MethodBody &os) {
   // a loop (parsers return FailureOr anyways).
   ArrayRef<AttrOrTypeParameter> params = def.getParameters();
   for (const AttrOrTypeParameter &param : params) {
+    if (isa<AttributeSelfTypeParameter>(param))
+      continue;
     os << formatv("::mlir::FailureOr<{0}> _result_{1};\n",
                   param.getCppStorageType(), param.getName());
   }
@@ -277,10 +279,9 @@ void DefFormat::genParser(MethodBody &os) {
   // Emit an assert for each mandatory parameter. Triggering an assert means
   // the generated parser is incorrect (i.e. there is a bug in this code).
   for (const AttrOrTypeParameter &param : params) {
-    if (!param.isOptional()) {
-      os << formatv("assert(::mlir::succeeded(_result_{0}));\n",
-                    param.getName());
-    }
+    if (param.isOptional() || isa<AttributeSelfTypeParameter>(param))
+      continue;
+    os << formatv("assert(::mlir::succeeded(_result_{0}));\n", param.getName());
   }
 
   // Generate call to the attribute or type builder. Use the checked getter
@@ -293,15 +294,18 @@ void DefFormat::genParser(MethodBody &os) {
                 def.getCppClassName());
   }
   for (const AttrOrTypeParameter &param : params) {
+    os << ",\n    ";
     if (param.isOptional()) {
-      os << formatv(",\n    _result_{0}.getValueOr(", param.getName());
+      os << formatv("_result_{0}.getValueOr(", param.getName());
       if (Optional<StringRef> defaultValue = param.getDefaultValue())
         os << tgfmt(*defaultValue, &ctx);
       else
         os << param.getCppStorageType() << "()";
       os << ")";
+    } else if (isa<AttributeSelfTypeParameter>(param)) {
+      os << tgfmt("$_type", &ctx);
     } else {
-      os << formatv(",\n    *_result_{0}", param.getName());
+      os << formatv("*_result_{0}", param.getName());
     }
   }
   os << ");";
@@ -666,7 +670,7 @@ void DefFormat::genPrinter(MethodBody &os) {
   ctx.addSubst("_ctx", "getContext()");
   os.indent();
 
-  /// Generate printers.
+  // Generate printers.
   shouldEmitSpace = true;
   lastWasPunctuation = false;
   for (FormatElement *el : elements)
@@ -904,10 +908,18 @@ LogicalResult DefFormatParser::verify(SMLoc loc,
                                       ArrayRef<FormatElement *> elements) {
   // Check that all parameters are referenced in the format.
   for (auto &it : llvm::enumerate(def.getParameters())) {
-    if (!it.value().isOptional() && !seenParams.test(it.index())) {
+    if (it.value().isOptional())
+      continue;
+    if (!seenParams.test(it.index())) {
+      if (isa<AttributeSelfTypeParameter>(it.value()))
+        continue;
       return emitError(loc, "format is missing reference to parameter: " +
                                 it.value().getName());
     }
+    if (isa<AttributeSelfTypeParameter>(it.value())) {
+      return emitError(loc,
+                       "unexpected self type parameter in assembly format");
+    }
   }
   if (elements.empty())
     return success();