[Mlir-commits] [mlir] 0533253 - [mlir][ods] Ignore AttributeSelfTypeParameter in assembly formats

llvmlistbot at llvm.org llvmlistbot at llvm.org
Mon May 16 13:23:59 PDT 2022


Author: Mogball
Date: 2022-05-16T20:23:54Z
New Revision: 0533253d81d8ab2641549963c7ad37dfccd39734

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

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

Added: 
    

Modified: 
    mlir/lib/IR/AsmPrinter.cpp
    mlir/test/lib/Dialect/Test/TestAttrDefs.td
    mlir/test/lib/Dialect/Test/TestAttributes.cpp
    mlir/test/mlir-tblgen/attr-or-type-format-roundtrip.mlir
    mlir/test/mlir-tblgen/attr-or-type-format.td
    mlir/test/mlir-tblgen/testdialect-attrdefs.mlir
    mlir/tools/mlir-tblgen/AttrOrTypeFormatGen.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index c9c7d4b103c25..e4d5c6d91459e 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/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>()) {

diff  --git a/mlir/test/lib/Dialect/Test/TestAttrDefs.td b/mlir/test/lib/Dialect/Test/TestAttrDefs.td
index fc2ce63a22b67..d62605d6eb3b8 100644
--- a/mlir/test/lib/Dialect/Test/TestAttrDefs.td
+++ b/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

diff  --git a/mlir/test/lib/Dialect/Test/TestAttributes.cpp b/mlir/test/lib/Dialect/Test/TestAttributes.cpp
index 45f6599059ca6..bc9a50bfeec40 100644
--- a/mlir/test/lib/Dialect/Test/TestAttributes.cpp
+++ b/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
 //===----------------------------------------------------------------------===//

diff  --git a/mlir/test/mlir-tblgen/attr-or-type-format-roundtrip.mlir b/mlir/test/mlir-tblgen/attr-or-type-format-roundtrip.mlir
index 894cd767d66a7..6ed246601468b 100644
--- a/mlir/test/mlir-tblgen/attr-or-type-format-roundtrip.mlir
+++ b/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

diff  --git a/mlir/test/mlir-tblgen/attr-or-type-format.td b/mlir/test/mlir-tblgen/attr-or-type-format.td
index eaa6ea3d32d63..b84ab8a667f57 100644
--- a/mlir/test/mlir-tblgen/attr-or-type-format.td
+++ b/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.
 

diff  --git a/mlir/test/mlir-tblgen/testdialect-attrdefs.mlir b/mlir/test/mlir-tblgen/testdialect-attrdefs.mlir
index 1e1cd31baf2c8..33fd88b45f99c 100644
--- a/mlir/test/mlir-tblgen/testdialect-attrdefs.mlir
+++ b/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()

diff  --git a/mlir/tools/mlir-tblgen/AttrOrTypeFormatGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeFormatGen.cpp
index 0c314b33caf82..17de304b4dbb4 100644
--- a/mlir/tools/mlir-tblgen/AttrOrTypeFormatGen.cpp
+++ b/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();


        


More information about the Mlir-commits mailing list