Skip to content

Commit f27c8ba

Browse files
committed
[MLIR] Remove spurious space when printing prop-dict
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 947e072 commit f27c8ba

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
@@ -207,6 +207,9 @@ class AsmPrinter {
207207
/// provide a valid type for the attribute.
208208
virtual void printAttributeWithoutType(Attribute attr);
209209

210+
/// Print the given named attribute.
211+
virtual void printNamedAttribute(NamedAttribute attr);
212+
210213
/// Print the alias for the given attribute, return failure if no alias could
211214
/// be printed.
212215
virtual LogicalResult printAlias(Attribute attr);

mlir/lib/IR/AsmPrinter.cpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,7 @@ class AsmPrinter::Impl {
442442
/// Print the given attribute without considering an alias.
443443
void printAttributeImpl(Attribute attr,
444444
AttrTypeElision typeElision = AttrTypeElision::Never);
445+
void printNamedAttribute(NamedAttribute attr);
445446

446447
/// Print the alias for the given attribute, return failure if no alias could
447448
/// be printed.
@@ -481,7 +482,6 @@ class AsmPrinter::Impl {
481482
void printOptionalAttrDict(ArrayRef<NamedAttribute> attrs,
482483
ArrayRef<StringRef> elidedAttrs = {},
483484
bool withKeyword = false);
484-
void printNamedAttribute(NamedAttribute attr);
485485
void printTrailingLocation(Location loc, bool allowAlias = true);
486486
void printLocationInternal(LocationAttr loc, bool pretty = false,
487487
bool isTopLevel = false);
@@ -801,6 +801,10 @@ class DummyAliasOperationPrinter : private OpAsmPrinter {
801801
void printAttributeWithoutType(Attribute attr) override {
802802
printAttribute(attr);
803803
}
804+
void printNamedAttribute(NamedAttribute attr) override {
805+
printAttribute(attr.getValue());
806+
}
807+
804808
LogicalResult printAlias(Attribute attr) override {
805809
initializer.visit(attr);
806810
return success();
@@ -975,6 +979,10 @@ class DummyAliasDialectAsmPrinter : public DialectAsmPrinter {
975979
recordAliasResult(
976980
initializer.visit(attr, canBeDeferred, /*elideType=*/true));
977981
}
982+
void printNamedAttribute(NamedAttribute attr) override {
983+
printAttribute(attr.getValue());
984+
}
985+
978986
LogicalResult printAlias(Attribute attr) override {
979987
printAttribute(attr);
980988
return success();
@@ -2381,7 +2389,6 @@ void AsmPrinter::Impl::printAttribute(Attribute attr,
23812389
return;
23822390
return printAttributeImpl(attr, typeElision);
23832391
}
2384-
23852392
void AsmPrinter::Impl::printAttributeImpl(Attribute attr,
23862393
AttrTypeElision typeElision) {
23872394
if (!isa<BuiltinDialect>(attr.getDialect())) {
@@ -2973,6 +2980,11 @@ void AsmPrinter::printAttributeWithoutType(Attribute attr) {
29732980
impl->printAttribute(attr, Impl::AttrTypeElision::Must);
29742981
}
29752982

2983+
void AsmPrinter::printNamedAttribute(NamedAttribute attr) {
2984+
assert(impl && "expected AsmPrinter::printNamedAttribute to be overriden");
2985+
impl->printNamedAttribute(attr);
2986+
}
2987+
29762988
void AsmPrinter::printKeywordOrString(StringRef keyword) {
29772989
assert(impl && "expected AsmPrinter::printKeywordOrString to be overriden");
29782990
::printKeywordOrString(keyword, impl->getStream());

mlir/lib/IR/Operation.cpp

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -810,13 +810,16 @@ void OpState::genericPrintProperties(OpAsmPrinter &p, Attribute properties,
810810
ArrayRef<NamedAttribute> attrs = dictAttr.getValue();
811811
llvm::SmallDenseSet<StringRef> elidedAttrsSet(elidedProps.begin(),
812812
elidedProps.end());
813-
bool atLeastOneAttr = llvm::any_of(attrs, [&](NamedAttribute attr) {
814-
return !elidedAttrsSet.contains(attr.getName().strref());
815-
});
816-
if (atLeastOneAttr) {
817-
p << "<";
818-
p.printOptionalAttrDict(dictAttr.getValue(), elidedProps);
819-
p << ">";
813+
auto filteredAttrs =
814+
llvm::make_filter_range(attrs, [&](NamedAttribute attr) {
815+
return !elidedAttrsSet.contains(attr.getName().strref());
816+
});
817+
if (!filteredAttrs.empty()) {
818+
p << "<{";
819+
interleaveComma(filteredAttrs, p, [&](NamedAttribute attr) {
820+
p.printNamedAttribute(attr);
821+
});
822+
p << "}>";
820823
}
821824
} else {
822825
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 < {packed, rhs = 16 : i64}>
519+
// CHECK: test.with_properties_and_inferred_type 16 <{packed, rhs = 16 : i64}>
520520
%should_be_i32 = test.with_properties_and_inferred_type 16 <{packed, 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)