[MLIR] Remove spurious space when printing prop-dict

[joker-eph]

When there is an elided properties, there use to be an extra space insert in the prop-dict printing before the dictionnary.

Fix #145695

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Mehdi Amini (joker-eph)

Changes

When there is an elided properties, there use to be an extra space insert in the prop-dict printing before the dictionnary.

Fix #145695

---

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

4 Files Affected:

• (modified) mlir/include/mlir/IR/OpImplementation.h (+3)
• (modified) mlir/lib/IR/AsmPrinter.cpp (+14-2)
• (modified) mlir/lib/IR/Operation.cpp (+9-3)
• (modified) mlir/test/mlir-tblgen/op-format.mlir (+5-5)

diff --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h
index 8710b970e8d75..d70aa346eaa1f 100644
--- a/mlir/include/mlir/IR/OpImplementation.h
+++ b/mlir/include/mlir/IR/OpImplementation.h
@@ -207,6 +207,9 @@ class AsmPrinter {
   /// provide a valid type for the attribute.
   virtual void printAttributeWithoutType(Attribute attr);
 
+  /// Print the given named attribute.
+  virtual void printNamedAttribute(NamedAttribute attr);
+
   /// Print the alias for the given attribute, return failure if no alias could
   /// be printed.
   virtual LogicalResult printAlias(Attribute attr);
diff --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index c7cc6a02ad208..748ce1d9f0c38 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/mlir/lib/IR/AsmPrinter.cpp
@@ -442,6 +442,7 @@ class AsmPrinter::Impl {
   /// Print the given attribute without considering an alias.
   void printAttributeImpl(Attribute attr,
                           AttrTypeElision typeElision = AttrTypeElision::Never);
+  void printNamedAttribute(NamedAttribute attr);
 
   /// Print the alias for the given attribute, return failure if no alias could
   /// be printed.
@@ -481,7 +482,6 @@ class AsmPrinter::Impl {
   void printOptionalAttrDict(ArrayRef<NamedAttribute> attrs,
                              ArrayRef<StringRef> elidedAttrs = {},
                              bool withKeyword = false);
-  void printNamedAttribute(NamedAttribute attr);
   void printTrailingLocation(Location loc, bool allowAlias = true);
   void printLocationInternal(LocationAttr loc, bool pretty = false,
                              bool isTopLevel = false);
@@ -801,6 +801,10 @@ class DummyAliasOperationPrinter : private OpAsmPrinter {
   void printAttributeWithoutType(Attribute attr) override {
     printAttribute(attr);
   }
+  void printNamedAttribute(NamedAttribute attr) override {
+    printAttribute(attr.getValue());
+  }
+
   LogicalResult printAlias(Attribute attr) override {
     initializer.visit(attr);
     return success();
@@ -975,6 +979,10 @@ class DummyAliasDialectAsmPrinter : public DialectAsmPrinter {
     recordAliasResult(
         initializer.visit(attr, canBeDeferred, /*elideType=*/true));
   }
+  void printNamedAttribute(NamedAttribute attr) override {
+    printAttribute(attr.getValue());
+  }
+
   LogicalResult printAlias(Attribute attr) override {
     printAttribute(attr);
     return success();
@@ -2381,7 +2389,6 @@ void AsmPrinter::Impl::printAttribute(Attribute attr,
     return;
   return printAttributeImpl(attr, typeElision);
 }
-
 void AsmPrinter::Impl::printAttributeImpl(Attribute attr,
                                           AttrTypeElision typeElision) {
   if (!isa<BuiltinDialect>(attr.getDialect())) {
@@ -2973,6 +2980,11 @@ void AsmPrinter::printAttributeWithoutType(Attribute attr) {
   impl->printAttribute(attr, Impl::AttrTypeElision::Must);
 }
 
+void AsmPrinter::printNamedAttribute(NamedAttribute attr) {
+  assert(impl && "expected AsmPrinter::printNamedAttribute to be overriden");
+  impl->printNamedAttribute(attr);
+}
+
 void AsmPrinter::printKeywordOrString(StringRef keyword) {
   assert(impl && "expected AsmPrinter::printKeywordOrString to be overriden");
   ::printKeywordOrString(keyword, impl->getStream());
diff --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp
index a479ff97f3f16..f61259917ee4d 100644
--- a/mlir/lib/IR/Operation.cpp
+++ b/mlir/lib/IR/Operation.cpp
@@ -814,9 +814,15 @@ void OpState::genericPrintProperties(OpAsmPrinter &p, Attribute properties,
       return !elidedAttrsSet.contains(attr.getName().strref());
     });
     if (atLeastOneAttr) {
-      p << "<";
-      p.printOptionalAttrDict(dictAttr.getValue(), elidedProps);
-      p << ">";
+      auto filteredAttrs =
+          llvm::make_filter_range(attrs, [&](NamedAttribute attr) {
+            return !elidedAttrsSet.contains(attr.getName().strref());
+          });
+      p << "<{";
+      interleaveComma(filteredAttrs, p, [&](NamedAttribute attr) {
+        p.printNamedAttribute(attr);
+      });
+      p << "}>";
     }
   } else {
     p << "<" << properties << ">";
diff --git a/mlir/test/mlir-tblgen/op-format.mlir b/mlir/test/mlir-tblgen/op-format.mlir
index 981fb5aff2aa2..fa929c2df9d10 100644
--- a/mlir/test/mlir-tblgen/op-format.mlir
+++ b/mlir/test/mlir-tblgen/op-format.mlir
@@ -290,10 +290,10 @@ test.format_optional_prop_dict <{a = [], b = 1 : i32}>
 // CHECK: test.format_optional_prop_dict {{$}}
 test.format_optional_prop_dict <{}>
 
-// CHECK: test.format_optional_prop_dict < {a = ["foo"]}>
+// CHECK: test.format_optional_prop_dict <{a = ["foo"]}>
 test.format_optional_prop_dict <{a = ["foo"]}>
 
-// CHECK: test.format_optional_prop_dict < {b = 2 : i32}>
+// CHECK: test.format_optional_prop_dict <{b = 2 : i32}>
 test.format_optional_prop_dict <{b = 2 : i32}>
 
 // CHECK: test.format_optional_prop_dict <{a = ["foo"], b = 2 : i32}>
@@ -513,15 +513,15 @@ test.format_infer_variadic_type_from_non_variadic %i64, %i64 : i64
 // CHECK: test.format_infer_type_variadic_operands(%[[I32]], %[[I32]] : i32, i32) (%[[I64]], %[[I64]] : i64, i64)
 %ignored_res13:4 = test.format_infer_type_variadic_operands(%i32, %i32 : i32, i32) (%i64, %i64 : i64, i64)
 
-// CHECK: test.with_properties_and_attr 16 < {rhs = 16 : i64}>
+// CHECK: test.with_properties_and_attr 16 <{rhs = 16 : i64}>
 test.with_properties_and_attr 16 <{rhs = 16 : i64}>
 
-// CHECK: test.with_properties_and_inferred_type 16 < {packed, rhs = 16 : i64}>
+// CHECK: test.with_properties_and_inferred_type 16 <{packed, rhs = 16 : i64}>
 %should_be_i32 = test.with_properties_and_inferred_type 16 <{packed, rhs = 16 : i64}>
 // Assert through the verifier that its inferred as i32.
 test.format_all_types_match_var %should_be_i32, %i32 : i32
 
-// CHECK: test.using_property_in_custom_and_other [1, 4, 20] < {other = 16 : i64}>
+// CHECK: test.using_property_in_custom_and_other [1, 4, 20] <{other = 16 : i64}>
 test.using_property_in_custom_and_other [1, 4, 20] <{other = 16 : i64}>
 
 //===----------------------------------------------------------------------===//

[Copilot]

Pull Request Overview

This PR fixes #145695 by removing the extra space when printing elided property dictionaries and refactors the printing logic to use an explicit comma-separated list wrapped in <{...}>. It also introduces a new printNamedAttribute hook in the AsmPrinter interface and its implementations.

• Refactor genericPrintProperties in Operation.cpp to build a filtered range and use interleaveComma inside <{ ... }>.
• Add printNamedAttribute declaration in OpImplementation.h and its free-function/Impl implementation in AsmPrinter.cpp, removing a duplicate declaration.
• Ensure all dialect/printer shims (DummyAliasOperationPrinter, DummyAliasDialectAsmPrinter) provide an override for printNamedAttribute.

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File	Description
mlir/lib/IR/Operation.cpp	Replace printOptionalAttrDict call with filtered range + interleaveComma and <{...}>.
mlir/lib/IR/AsmPrinter.cpp	Add AsmPrinter::printNamedAttribute free function; implement in Impl and remove duplicate declaration.
mlir/include/mlir/IR/OpImplementation.h	Declare virtual void printNamedAttribute(NamedAttribute) in AsmPrinter interface.