Skip to content

Commit

Permalink
[mlir] fix bugs with NamedAttrList
Browse files Browse the repository at this point in the history
- `assign` with ArrayRef was calling `append`
- `assign` with empty ArrayRef was not clearing storage

Reviewed By: jpienaar

Differential Revision: https://reviews.llvm.org/D112043
  • Loading branch information
Mogball committed Oct 19, 2021
1 parent bc03a9c commit 21bb463
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 1 deletion.
2 changes: 1 addition & 1 deletion mlir/include/mlir/IR/OperationSupport.h
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ class NamedAttrList {

/// Replaces the attributes with new list of attributes.
void assign(ArrayRef<NamedAttribute> range) {
append(range.begin(), range.end());
assign(range.begin(), range.end());
}

bool empty() const { return attrs.empty(); }
Expand Down
2 changes: 2 additions & 0 deletions mlir/lib/IR/BuiltinAttributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ static bool dictionaryAttrSort(ArrayRef<NamedAttribute> value,
switch (value.size()) {
case 0:
// Zero already sorted.
if (!inPlace)
storage.clear();
break;
case 1:
// One already sorted but may need to be copied.
Expand Down
44 changes: 44 additions & 0 deletions mlir/unittests/IR/OperationSupportTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,4 +225,48 @@ TEST(OperationFormatPrintTest, CanUseVariadicFormat) {
ASSERT_STREQ(str.c_str(), "\"foo.bar\"() : () -> ()");
}

TEST(NamedAttrListTest, TestAppendAssign) {
MLIRContext ctx;
NamedAttrList attrs;
Builder b(&ctx);

attrs.append("foo", b.getStringAttr("bar"));
attrs.append("baz", b.getStringAttr("boo"));

{
auto it = attrs.begin();
EXPECT_EQ(it->first, b.getIdentifier("foo"));
EXPECT_EQ(it->second, b.getStringAttr("bar"));
++it;
EXPECT_EQ(it->first, b.getIdentifier("baz"));
EXPECT_EQ(it->second, b.getStringAttr("boo"));
}

attrs.append("foo", b.getStringAttr("zoo"));
{
auto dup = attrs.findDuplicate();
ASSERT_TRUE(dup.hasValue());
}

SmallVector<NamedAttribute> newAttrs = {
b.getNamedAttr("foo", b.getStringAttr("f")),
b.getNamedAttr("zoo", b.getStringAttr("z")),
};
attrs.assign(newAttrs);

auto dup = attrs.findDuplicate();
ASSERT_FALSE(dup.hasValue());

{
auto it = attrs.begin();
EXPECT_EQ(it->first, b.getIdentifier("foo"));
EXPECT_EQ(it->second, b.getStringAttr("f"));
++it;
EXPECT_EQ(it->first, b.getIdentifier("zoo"));
EXPECT_EQ(it->second, b.getStringAttr("z"));
}

attrs.assign({});
ASSERT_TRUE(attrs.empty());
}
} // end namespace

0 comments on commit 21bb463

Please sign in to comment.