Skip to content

Commit 7356ad1

Browse files
Avoid copies in getChecked (cherry-pick from upstream) (#151)
Cherry-pick from upstream llvm: llvm/llvm-project#147721; adding std::move to getChecked method.
1 parent a6570ab commit 7356ad1

File tree

6 files changed

+38
-4
lines changed

6 files changed

+38
-4
lines changed

mlir/include/mlir/IR/StorageUniquerSupport.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ class StorageUserBase : public BaseT, public Traits<ConcreteT>... {
200200
// If the construction invariants fail then we return a null attribute.
201201
if (failed(ConcreteT::verify(emitErrorFn, args...)))
202202
return ConcreteT();
203-
return UniquerT::template get<ConcreteT>(ctx, args...);
203+
return UniquerT::template get<ConcreteT>(ctx, std::forward<Args>(args)...);
204204
}
205205

206206
/// Get an instance of the concrete type from a void pointer.

mlir/test/lib/Dialect/Test/TestAttrDefs.td

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,7 @@ def TestCopyCount : Test_Attr<"TestCopyCount"> {
331331
let mnemonic = "copy_count";
332332
let parameters = (ins TestParamCopyCount:$copy_count);
333333
let assemblyFormat = "`<` $copy_count `>`";
334+
let genVerifyDecl = 1;
334335
}
335336

336337
def TestConditionalAliasAttr : Test_Attr<"TestConditionalAlias"> {

mlir/test/lib/Dialect/Test/TestAttributes.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,16 @@ static void printTrueFalse(AsmPrinter &p, std::optional<int> result) {
177177
p << (*result ? "true" : "false");
178178
}
179179

180+
//===----------------------------------------------------------------------===//
181+
// TestCopyCountAttr Implementation
182+
//===----------------------------------------------------------------------===//
183+
184+
LogicalResult TestCopyCountAttr::verify(
185+
llvm::function_ref<::mlir::InFlightDiagnostic()> /*emitError*/,
186+
CopyCount /*copy_count*/) {
187+
return success();
188+
}
189+
180190
//===----------------------------------------------------------------------===//
181191
// CopyCountAttr Implementation
182192
//===----------------------------------------------------------------------===//

mlir/test/mlir-tblgen/attrdefs.td

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,11 @@ def B_CompoundAttrA : TestAttr<"CompoundA"> {
115115
// DEF: return new (allocator.allocate<CompoundAAttrStorage>())
116116
// DEF-SAME: CompoundAAttrStorage(std::move(widthOfSomething), std::move(exampleTdType), std::move(apFloat), std::move(dims), std::move(inner));
117117

118+
// DEF: CompoundAAttr CompoundAAttr::getChecked(
119+
// DEF-SAME: int widthOfSomething, ::test::SimpleTypeA exampleTdType, ::llvm::APFloat apFloat, ::llvm::ArrayRef<int> dims, ::mlir::Type inner
120+
// DEF-SAME: )
121+
// DEF-NEXT: return Base::getChecked(emitError, context, std::move(widthOfSomething), std::move(exampleTdType), std::move(apFloat), std::move(dims), std::move(inner));
122+
118123
// DEF: ::mlir::Type CompoundAAttr::getInner() const {
119124
// DEF-NEXT: return getImpl()->inner;
120125
}

mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ void DefGen::emitCheckedBuilder() {
393393
MethodBody &body = m->body().indent();
394394
auto scope = body.scope("return Base::getChecked(emitError, context", ");");
395395
for (const auto &param : params)
396-
body << ", " << param.getName();
396+
body << ", std::move(" << param.getName() << ")";
397397
}
398398

399399
static SmallVector<MethodParameter>

mlir/unittests/IR/AttributeTest.cpp

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -462,8 +462,9 @@ TEST(SubElementTest, Nested) {
462462
{strAttr, trueAttr, falseAttr, boolArrayAttr, dictAttr}));
463463
}
464464

465-
// Test how many times we call copy-ctor when building an attribute.
466-
TEST(CopyCountAttr, CopyCount) {
465+
// Test how many times we call copy-ctor when building an attribute with the
466+
// 'get' method.
467+
TEST(CopyCountAttr, CopyCountGet) {
467468
MLIRContext context;
468469
context.loadDialect<test::TestDialect>();
469470

@@ -483,6 +484,23 @@ TEST(CopyCountAttr, CopyCount) {
483484
#endif
484485
}
485486

487+
// Test how many times we call copy-ctor when building an attribute with the
488+
// 'getChecked' method.
489+
TEST(CopyCountAttr, CopyCountGetChecked) {
490+
MLIRContext context;
491+
context.loadDialect<test::TestDialect>();
492+
test::CopyCount::counter = 0;
493+
test::CopyCount copyCount("hello");
494+
auto loc = UnknownLoc::get(&context);
495+
test::TestCopyCountAttr::getChecked(loc, &context, std::move(copyCount));
496+
int counter1 = test::CopyCount::counter;
497+
test::CopyCount::counter = 0;
498+
test::TestCopyCountAttr::getChecked(loc, &context, std::move(copyCount));
499+
// One verification requires a copy.
500+
EXPECT_EQ(counter1, 1);
501+
EXPECT_EQ(test::CopyCount::counter, 1);
502+
}
503+
486504
// Test stripped printing using test dialect attribute.
487505
TEST(CopyCountAttr, PrintStripped) {
488506
MLIRContext context;

0 commit comments

Comments
 (0)