Skip to content

Commit b9ac6f5

Browse files
joker-ephandrey-golubev
authored andcommitted
[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
1 parent 6ccc228 commit b9ac6f5

File tree

4 files changed

+32
-14
lines changed

4 files changed

+32
-14
lines changed

mlir/include/mlir/IR/OpImplementation.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,9 @@ class AsmPrinter {
190190
/// provide a valid type for the attribute.
191191
virtual void printAttributeWithoutType(Attribute attr);
192192

193+
/// Print the given named attribute.
194+
virtual void printNamedAttribute(NamedAttribute attr);
195+
193196
/// Print the alias for the given attribute, return failure if no alias could
194197
/// be printed.
195198
virtual LogicalResult printAlias(Attribute attr);

mlir/lib/IR/AsmPrinter.cpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,7 @@ class AsmPrinter::Impl {
449449
/// Print the given attribute without considering an alias.
450450
void printAttributeImpl(Attribute attr,
451451
AttrTypeElision typeElision = AttrTypeElision::Never);
452+
void printNamedAttribute(NamedAttribute attr);
452453

453454
/// Print the alias for the given attribute, return failure if no alias could
454455
/// be printed.
@@ -488,7 +489,6 @@ class AsmPrinter::Impl {
488489
void printOptionalAttrDict(ArrayRef<NamedAttribute> attrs,
489490
ArrayRef<StringRef> elidedAttrs = {},
490491
bool withKeyword = false);
491-
void printNamedAttribute(NamedAttribute attr);
492492
void printTrailingLocation(Location loc, bool allowAlias = true);
493493
void printLocationInternal(LocationAttr loc, bool pretty = false,
494494
bool isTopLevel = false);
@@ -805,6 +805,10 @@ class DummyAliasOperationPrinter : private OpAsmPrinter {
805805
void printAttributeWithoutType(Attribute attr) override {
806806
printAttribute(attr);
807807
}
808+
void printNamedAttribute(NamedAttribute attr) override {
809+
printAttribute(attr.getValue());
810+
}
811+
808812
LogicalResult printAlias(Attribute attr) override {
809813
initializer.visit(attr);
810814
return success();
@@ -979,6 +983,10 @@ class DummyAliasDialectAsmPrinter : public DialectAsmPrinter {
979983
recordAliasResult(
980984
initializer.visit(attr, canBeDeferred, /*elideType=*/true));
981985
}
986+
void printNamedAttribute(NamedAttribute attr) override {
987+
printAttribute(attr.getValue());
988+
}
989+
982990
LogicalResult printAlias(Attribute attr) override {
983991
printAttribute(attr);
984992
return success();
@@ -2321,7 +2329,6 @@ void AsmPrinter::Impl::printAttribute(Attribute attr,
23212329
return;
23222330
return printAttributeImpl(attr, typeElision);
23232331
}
2324-
23252332
void AsmPrinter::Impl::printAttributeImpl(Attribute attr,
23262333
AttrTypeElision typeElision) {
23272334
if (!isa<BuiltinDialect>(attr.getDialect())) {
@@ -2940,6 +2947,11 @@ void AsmPrinter::printAttributeWithoutType(Attribute attr) {
29402947
impl->printAttribute(attr, Impl::AttrTypeElision::Must);
29412948
}
29422949

2950+
void AsmPrinter::printNamedAttribute(NamedAttribute attr) {
2951+
assert(impl && "expected AsmPrinter::printNamedAttribute to be overriden");
2952+
impl->printNamedAttribute(attr);
2953+
}
2954+
29432955
void AsmPrinter::printKeywordOrString(StringRef keyword) {
29442956
assert(impl && "expected AsmPrinter::printKeywordOrString to be overriden");
29452957
::printKeywordOrString(keyword, impl->getStream());

mlir/lib/IR/Operation.cpp

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -808,13 +808,16 @@ void OpState::genericPrintProperties(OpAsmPrinter &p, Attribute properties,
808808
ArrayRef<NamedAttribute> attrs = dictAttr.getValue();
809809
llvm::SmallDenseSet<StringRef> elidedAttrsSet(elidedProps.begin(),
810810
elidedProps.end());
811-
bool atLeastOneAttr = llvm::any_of(attrs, [&](NamedAttribute attr) {
812-
return !elidedAttrsSet.contains(attr.getName().strref());
813-
});
814-
if (atLeastOneAttr) {
815-
p << "<";
816-
p.printOptionalAttrDict(dictAttr.getValue(), elidedProps);
817-
p << ">";
811+
auto filteredAttrs =
812+
llvm::make_filter_range(attrs, [&](NamedAttribute attr) {
813+
return !elidedAttrsSet.contains(attr.getName().strref());
814+
});
815+
if (!filteredAttrs.empty()) {
816+
p << "<{";
817+
interleaveComma(filteredAttrs, p, [&](NamedAttribute attr) {
818+
p.printNamedAttribute(attr);
819+
});
820+
p << "}>";
818821
}
819822
} else {
820823
p << "<" << properties << ">";

mlir/test/mlir-tblgen/op-format.mlir

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -290,10 +290,10 @@ test.format_optional_prop_dict <{a = [], b = 1 : i32}>
290290
// CHECK: test.format_optional_prop_dict {{$}}
291291
test.format_optional_prop_dict <{}>
292292

293-
// CHECK: test.format_optional_prop_dict < {a = ["foo"]}>
293+
// CHECK: test.format_optional_prop_dict <{a = ["foo"]}>
294294
test.format_optional_prop_dict <{a = ["foo"]}>
295295

296-
// CHECK: test.format_optional_prop_dict < {b = 2 : i32}>
296+
// CHECK: test.format_optional_prop_dict <{b = 2 : i32}>
297297
test.format_optional_prop_dict <{b = 2 : i32}>
298298

299299
// 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
513513
// CHECK: test.format_infer_type_variadic_operands(%[[I32]], %[[I32]] : i32, i32) (%[[I64]], %[[I64]] : i64, i64)
514514
%ignored_res13:4 = test.format_infer_type_variadic_operands(%i32, %i32 : i32, i32) (%i64, %i64 : i64, i64)
515515

516-
// CHECK: test.with_properties_and_attr 16 < {rhs = 16 : i64}>
516+
// CHECK: test.with_properties_and_attr 16 <{rhs = 16 : i64}>
517517
test.with_properties_and_attr 16 <{rhs = 16 : i64}>
518518

519-
// CHECK: test.with_properties_and_inferred_type 16 < {rhs = 16 : i64}>
519+
// CHECK: test.with_properties_and_inferred_type 16 <{rhs = 16 : i64}>
520520
%should_be_i32 = test.with_properties_and_inferred_type 16 <{rhs = 16 : i64}>
521521
// Assert through the verifier that its inferred as i32.
522522
test.format_all_types_match_var %should_be_i32, %i32 : i32
523523

524-
// CHECK: test.using_property_in_custom_and_other [1, 4, 20] < {other = 16 : i64}>
524+
// CHECK: test.using_property_in_custom_and_other [1, 4, 20] <{other = 16 : i64}>
525525
test.using_property_in_custom_and_other [1, 4, 20] <{other = 16 : i64}>
526526

527527
//===----------------------------------------------------------------------===//

0 commit comments

Comments
 (0)