From c9ecabdfce50815c9b7e593bedba7971fba4e5db Mon Sep 17 00:00:00 2001 From: Alexandru Lorinti Date: Wed, 9 Jul 2025 16:12:20 +0300 Subject: [PATCH 1/4] avoid copies --- mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp index dbae2143b920a..3140f12c0b7e8 100644 --- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp +++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp @@ -495,7 +495,7 @@ void DefGen::emitCheckedBuilder() { MethodBody &body = m->body().indent(); auto scope = body.scope("return Base::getChecked(emitError, context", ");"); for (const auto ¶m : params) - body << ", " << param.getName(); + body << ", std::move(" << param.getName() << ")"; } static SmallVector From f214e2967fa3f2ed197a27774c6a1ab602431317 Mon Sep 17 00:00:00 2001 From: Alexandru Lorinti Date: Fri, 18 Jul 2025 05:35:47 +0300 Subject: [PATCH 2/4] updating tblgen test --- mlir/test/mlir-tblgen/attrdefs.td | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mlir/test/mlir-tblgen/attrdefs.td b/mlir/test/mlir-tblgen/attrdefs.td index d47411d6e860a..a809611fd0aec 100644 --- a/mlir/test/mlir-tblgen/attrdefs.td +++ b/mlir/test/mlir-tblgen/attrdefs.td @@ -115,6 +115,11 @@ def B_CompoundAttrA : TestAttr<"CompoundA"> { // DEF: return new (allocator.allocate()) // DEF-SAME: CompoundAAttrStorage(std::move(widthOfSomething), std::move(exampleTdType), std::move(apFloat), std::move(dims), std::move(inner)); +// DEF: CompoundAAttr CompoundAAttr::getChecked( +// DEF-SAME: int widthOfSomething, ::test::SimpleTypeA exampleTdType, ::llvm::APFloat apFloat, ::llvm::ArrayRef dims, ::mlir::Type inner +// DEF-SAME: ) +// DEF-NEXT: return Base::getChecked(emitError, context, std::move(widthOfSomething), std::move(exampleTdType), std::move(apFloat), std::move(dims), std::move(inner)); + // DEF: ::mlir::Type CompoundAAttr::getInner() const { // DEF-NEXT: return getImpl()->inner; } From 498f0941d000258259ee2e91665e5c54788f9830 Mon Sep 17 00:00:00 2001 From: Alexandru Lorinti Date: Tue, 22 Jul 2025 14:37:49 +0300 Subject: [PATCH 3/4] adding copy count test case for 'getChecked' method --- mlir/include/mlir/IR/StorageUniquerSupport.h | 2 +- mlir/test/lib/Dialect/Test/TestAttrDefs.td | 1 + mlir/test/lib/Dialect/Test/TestAttributes.cpp | 8 +++++ mlir/unittests/IR/AttributeTest.cpp | 31 ++++++++++++++++--- 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/mlir/include/mlir/IR/StorageUniquerSupport.h b/mlir/include/mlir/IR/StorageUniquerSupport.h index 2162a74a51580..8959dab047103 100644 --- a/mlir/include/mlir/IR/StorageUniquerSupport.h +++ b/mlir/include/mlir/IR/StorageUniquerSupport.h @@ -200,7 +200,7 @@ class StorageUserBase : public BaseT, public Traits... { // If the construction invariants fail then we return a null attribute. if (failed(ConcreteT::verifyInvariants(emitErrorFn, args...))) return ConcreteT(); - return UniquerT::template get(ctx, args...); + return UniquerT::template get(ctx, std::forward(args)...); } /// Get an instance of the concrete type from a void pointer. diff --git a/mlir/test/lib/Dialect/Test/TestAttrDefs.td b/mlir/test/lib/Dialect/Test/TestAttrDefs.td index 382da592d0079..5685004bbbd25 100644 --- a/mlir/test/lib/Dialect/Test/TestAttrDefs.td +++ b/mlir/test/lib/Dialect/Test/TestAttrDefs.td @@ -347,6 +347,7 @@ def TestCopyCount : Test_Attr<"TestCopyCount"> { let mnemonic = "copy_count"; let parameters = (ins TestParamCopyCount:$copy_count); let assemblyFormat = "`<` $copy_count `>`"; + let genVerifyDecl = 1; } def TestConditionalAliasAttr : Test_Attr<"TestConditionalAlias"> { diff --git a/mlir/test/lib/Dialect/Test/TestAttributes.cpp b/mlir/test/lib/Dialect/Test/TestAttributes.cpp index b31e90fc9ca91..d728d2c478d13 100644 --- a/mlir/test/lib/Dialect/Test/TestAttributes.cpp +++ b/mlir/test/lib/Dialect/Test/TestAttributes.cpp @@ -213,6 +213,14 @@ static void printTrueFalse(AsmPrinter &p, std::optional result) { p << (*result ? "true" : "false"); } +//===----------------------------------------------------------------------===// +// TestCopyCountAttr Implementation +//===----------------------------------------------------------------------===// + +LogicalResult TestCopyCountAttr::verify(llvm::function_ref<::mlir::InFlightDiagnostic()> /*emitError*/, CopyCount /*copy_count*/) { + return success(); +} + //===----------------------------------------------------------------------===// // CopyCountAttr Implementation //===----------------------------------------------------------------------===// diff --git a/mlir/unittests/IR/AttributeTest.cpp b/mlir/unittests/IR/AttributeTest.cpp index a55592db7132d..fd40404bf3008 100644 --- a/mlir/unittests/IR/AttributeTest.cpp +++ b/mlir/unittests/IR/AttributeTest.cpp @@ -477,8 +477,9 @@ TEST(SubElementTest, Nested) { {strAttr, trueAttr, falseAttr, boolArrayAttr, dictAttr})); } -// Test how many times we call copy-ctor when building an attribute. -TEST(CopyCountAttr, CopyCount) { +// Test how many times we call copy-ctor when building an attribute with the +// 'get' method. +TEST(CopyCountAttr, CopyCountGet) { MLIRContext context; context.loadDialect(); @@ -489,15 +490,35 @@ TEST(CopyCountAttr, CopyCount) { test::CopyCount::counter = 0; test::TestCopyCountAttr::get(&context, std::move(copyCount)); #ifndef NDEBUG - // One verification enabled only in assert-mode requires a copy. - EXPECT_EQ(counter1, 1); - EXPECT_EQ(test::CopyCount::counter, 1); + // One verification enabled only in assert-mode requires two copies: one for + // calling 'verifyInvariants' and one for calling 'verify' inside + // 'verifyInvariants'. + EXPECT_EQ(counter1, 2); + EXPECT_EQ(test::CopyCount::counter, 2); #else EXPECT_EQ(counter1, 0); EXPECT_EQ(test::CopyCount::counter, 0); #endif } +// Test how many times we call copy-ctor when building an attribute with the +// 'getChecked' method. +TEST(CopyCountAttr, CopyCountGetChecked) { + MLIRContext context; + context.loadDialect(); + test::CopyCount::counter = 0; + test::CopyCount copyCount("hello"); + auto loc = UnknownLoc::get(&context); + test::TestCopyCountAttr::getChecked(loc, &context, std::move(copyCount)); + int counter1 = test::CopyCount::counter; + test::CopyCount::counter = 0; + test::TestCopyCountAttr::getChecked(loc, &context, std::move(copyCount)); + // The verifiers require two copies: one for calling 'verifyInvariants' and + // one for calling 'verify' inside 'verifyInvariants'. + EXPECT_EQ(counter1, 2); + EXPECT_EQ(test::CopyCount::counter, 2); +} + // Test stripped printing using test dialect attribute. TEST(CopyCountAttr, PrintStripped) { MLIRContext context; From d20697c255a367fc14a6bf7a01ef758d12daf400 Mon Sep 17 00:00:00 2001 From: Alexandru Lorinti Date: Thu, 24 Jul 2025 06:01:05 +0300 Subject: [PATCH 4/4] fix formatting --- mlir/test/lib/Dialect/Test/TestAttributes.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mlir/test/lib/Dialect/Test/TestAttributes.cpp b/mlir/test/lib/Dialect/Test/TestAttributes.cpp index d728d2c478d13..58909131e50a3 100644 --- a/mlir/test/lib/Dialect/Test/TestAttributes.cpp +++ b/mlir/test/lib/Dialect/Test/TestAttributes.cpp @@ -217,7 +217,9 @@ static void printTrueFalse(AsmPrinter &p, std::optional result) { // TestCopyCountAttr Implementation //===----------------------------------------------------------------------===// -LogicalResult TestCopyCountAttr::verify(llvm::function_ref<::mlir::InFlightDiagnostic()> /*emitError*/, CopyCount /*copy_count*/) { +LogicalResult TestCopyCountAttr::verify( + llvm::function_ref<::mlir::InFlightDiagnostic()> /*emitError*/, + CopyCount /*copy_count*/) { return success(); }