From aeb656f08e1ba01ea2e2282bb30cb519bec4833c Mon Sep 17 00:00:00 2001 From: Mehdi Amini Date: Wed, 2 Jul 2025 14:07:17 +0200 Subject: [PATCH] [MLIR] Remove spurious space when printing `prop-dict` (#145962) When there is an elided properties, there use to be an extra space insert in the prop-dict printing before the dictionnary. Fix #145695 --- mlir/include/mlir/IR/OpImplementation.h | 3 +++ mlir/lib/IR/AsmPrinter.cpp | 16 ++++++++++++++-- mlir/lib/IR/Operation.cpp | 17 ++++++++++------- mlir/test/mlir-tblgen/op-format.mlir | 10 +++++----- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h index e4ab72217de2..104b34a33910 100644 --- a/mlir/include/mlir/IR/OpImplementation.h +++ b/mlir/include/mlir/IR/OpImplementation.h @@ -190,6 +190,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 4071e816a54f..bdf8da5b1540 100644 --- a/mlir/lib/IR/AsmPrinter.cpp +++ b/mlir/lib/IR/AsmPrinter.cpp @@ -449,6 +449,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. @@ -488,7 +489,6 @@ class AsmPrinter::Impl { void printOptionalAttrDict(ArrayRef attrs, ArrayRef 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); @@ -805,6 +805,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(); @@ -979,6 +983,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(); @@ -2321,7 +2329,6 @@ void AsmPrinter::Impl::printAttribute(Attribute attr, return; return printAttributeImpl(attr, typeElision); } - void AsmPrinter::Impl::printAttributeImpl(Attribute attr, AttrTypeElision typeElision) { if (!isa(attr.getDialect())) { @@ -2940,6 +2947,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 fe0fee0f8db2..25f3cfd37b47 100644 --- a/mlir/lib/IR/Operation.cpp +++ b/mlir/lib/IR/Operation.cpp @@ -808,13 +808,16 @@ void OpState::genericPrintProperties(OpAsmPrinter &p, Attribute properties, ArrayRef attrs = dictAttr.getValue(); llvm::SmallDenseSet elidedAttrsSet(elidedProps.begin(), elidedProps.end()); - bool atLeastOneAttr = llvm::any_of(attrs, [&](NamedAttribute attr) { - 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()); + }); + if (!filteredAttrs.empty()) { + 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 08b0c52413a7..6f3cd861716d 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 < {rhs = 16 : i64}> +// CHECK: test.with_properties_and_inferred_type 16 <{rhs = 16 : i64}> %should_be_i32 = test.with_properties_and_inferred_type 16 <{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}> //===----------------------------------------------------------------------===//