[mlir] [attribute] Fix inconsistent nested attribute parser and printer

[JueonPark]

This PR synchronizes the parsing & printing format of attributes & types, especially for nested types. It also contains a reproduction test.
See https://discourse.llvm.org/t/inconsistent-printer-and-parser-behavior-for-nested-attributes-in-mlir-tablegen/85585 for related discussion.

Changes

This PR forces genCommaSeparatedPrinter to set shouldBeQualifiedFlag if it needs to be. This would fix the nested parser & printer to accept the same format, especially for attributes with struct(params) assembly format.

For reproduction, it implements three attributes, which are:

• An internal attribute (InternalAttr) that would be the parameter of external attributes,
• An external attribute (ExternalAttr) that takes one internal attribute parameter, and
• An external attribute (ExternalArrayAttr) that takes an array of internal attribute.

As shown in mlir/test/mlir-tblgen/op-format.mlir, this PR now enables both parser & printer to accept the same format.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-mlir-core

@llvm/pr-subscribers-mlir

Author: Jueon Park (JueonPark)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/133872.diff

2 Files Affected:

• (modified) mlir/test/lib/Dialect/Test/TestAttrDefs.td (+30)
• (modified) mlir/test/mlir-tblgen/op-format.mlir (+21)

diff --git a/mlir/test/lib/Dialect/Test/TestAttrDefs.td b/mlir/test/lib/Dialect/Test/TestAttrDefs.td
index fc2d77af29f12..6282154b9b912 100644
--- a/mlir/test/lib/Dialect/Test/TestAttrDefs.td
+++ b/mlir/test/lib/Dialect/Test/TestAttrDefs.td
@@ -58,6 +58,36 @@ def CompoundAttrNested : Test_Attr<"CompoundAttrNested"> {
   let assemblyFormat = "`<` `nested` `=` $nested `>`";
 }
 
+// Nested attributes for reproducing.
+def InternalAttr : Test_Attr<"Internal"> {
+  let mnemonic = "internal";
+
+  let parameters = (ins
+    "int64_t":$key,
+    "int64_t":$value
+  );
+
+  let assemblyFormat = "`<` struct(params) `>`";
+}
+
+def ExternalAttr : Test_Attr<"External"> {
+  let mnemonic = "external";
+
+  let parameters = (ins InternalAttr:$internal);
+
+  let assemblyFormat = "`<` struct(params) `>`";
+}
+
+def ExternalArrayAttr : Test_Attr<"ExternalArray"> {
+  let mnemonic = "external_array";
+
+  let parameters = (ins
+    ArrayRefParameter<"InternalAttr">:$internals
+  );
+
+  let assemblyFormat = "`<` `[` struct(params) `]` `>`";
+}
+
 // An attribute testing AttributeSelfTypeParameter.
 def AttrWithSelfTypeParam
     : Test_Attr<"AttrWithSelfTypeParam", [TypedAttrInterface]> {
diff --git a/mlir/test/mlir-tblgen/op-format.mlir b/mlir/test/mlir-tblgen/op-format.mlir
index 08b0c52413a75..7d0bbd8a4273a 100644
--- a/mlir/test/mlir-tblgen/op-format.mlir
+++ b/mlir/test/mlir-tblgen/op-format.mlir
@@ -345,6 +345,27 @@ module attributes {test.someAttr = #test.cmpnd_nested_outer<i <42 <1, !test.smpl
 
 //-----
 
+// CHECK: module attributes {test.internal = #test.internal<key = 8, value = 9>} {
+// CHECK-NEXT: }
+module attributes {test.internal = #test.internal<key = 8, value = 9>} {
+}
+
+//-----
+
+// CHECK: module attributes {test.external = #test.external<internal = <key = 1, value = 2>>} {
+// CHECK-NEXT: }
+module attributes {test.external = #test.external<internal = #test.internal<key = 1, value = 2>>} {
+}
+
+//-----
+
+// CHECK: module attributes {test.external_array = #test.external_array<[internals = <key = 1, value = 2>, <key = 8, value = 9>]>} {
+// CHECK-NEXT: }
+module attributes {test.external_array = #test.external_array<[internals = #test.internal<key = 1, value = 2>, #test.internal<key = 8, value = 9>]>} {
+}
+
+//-----
+
 // CHECK: test.format_cpmd_nested_attr nested <i <42 <1, !test.smpla, [5, 6]>>>
 test.format_cpmd_nested_attr nested <i <42 <1, !test.smpla, [5, 6]>>>
 

[joker-eph]

Where is the nested qualified print here?

[JueonPark]

The nested qualified only prints the first qualified mnemonic.

Before:

module attributes {test.external_array = #test.external_array<[internals = #test.internal<key = 1, value = 2>, #test.internal<key = 8, value = 9>]>} {
}

After:

module attributes {test.external_array = #test.external_array<[internals = <key = 1, value = 2>, <key = 8, value = 9>]>} {
}

[JueonPark]

I believe I may have misunderstood.

Initially, I thought the current parser always required all nested attributes to be qualified. Therefore, I assumed the printer also needed to print all nested attributes as qualified.

This was incorrect. The current parser was already parsing unqualified nested attributes correctly and was in sync with the printer.

Since this patch doesn't create different logic for the parser and the printer in the first place, I think this PR could be closed.
Thank you for your time reviewing this, @joker-eph .