[Mlir-commits] [mlir] [mlir][ODS] Fix TableGen output for Attr::hasStorageCustomConstructor (PR #147957)

[Andrei Golubev]

https://github.com/andrey-golubev updated https://github.com/llvm/llvm-project/pull/147957

>From 916e3941833da6f6d67a63a1fbc7f5e5313d5fb8 Mon Sep 17 00:00:00 2001
From: "Golubev, Andrey" <andrey.golubev at intel.com>
Date: Thu, 10 Jul 2025 13:15:41 +0000
Subject: [PATCH 1/2] [mlir][ODS] Fix TableGen output for
 Attr::hasStorageCustomConstructor

There is a `hasStorageCustomConstructor` flag that allows one to provide
custom attribute construction implementation. Unfortunately, it seems
like the flag does not work properly: the generated C++ produces *empty
body* method instead of producing only a declaration.
---
 mlir/test/lib/Dialect/Test/TestAttrDefs.td    |  7 +++++++
 mlir/test/lib/Dialect/Test/TestAttributes.cpp | 12 ++++++++++++
 mlir/test/mlir-tblgen/attrdefs.td             | 13 +++++++++++++
 mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp   |  4 ++--
 4 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/mlir/test/lib/Dialect/Test/TestAttrDefs.td b/mlir/test/lib/Dialect/Test/TestAttrDefs.td
index 4d825e2f0a8cc..382da592d0079 100644
--- a/mlir/test/lib/Dialect/Test/TestAttrDefs.td
+++ b/mlir/test/lib/Dialect/Test/TestAttrDefs.td
@@ -431,4 +431,11 @@ def SlashAttr: Test_Attr<"Slash">{
   let hasCustomAssemblyFormat = 1;
 }
 
+def TestCustomStorageCtorAttr : Test_Attr<"TestCustomStorageCtorAttr"> {
+    let mnemonic = "custom_storage_ctor_attr";
+    let parameters = (ins "int":$value);
+    let assemblyFormat = "`<` $value `>`";
+    let hasStorageCustomConstructor = 1;
+}
+
 #endif // TEST_ATTRDEFS
diff --git a/mlir/test/lib/Dialect/Test/TestAttributes.cpp b/mlir/test/lib/Dialect/Test/TestAttributes.cpp
index 4f6655d0b2978..b31e90fc9ca91 100644
--- a/mlir/test/lib/Dialect/Test/TestAttributes.cpp
+++ b/mlir/test/lib/Dialect/Test/TestAttributes.cpp
@@ -515,6 +515,18 @@ void SlashAttr::print(AsmPrinter &printer) const {
   printer << "<" << getLhs() << " / " << getRhs() << ">";
 }
 
+//===----------------------------------------------------------------------===//
+// TestCustomStorageCtorAttr
+//===----------------------------------------------------------------------===//
+
+test::detail::TestCustomStorageCtorAttrAttrStorage *
+test::detail::TestCustomStorageCtorAttrAttrStorage::construct(
+    mlir::StorageUniquer::StorageAllocator &, std::tuple<int> &&) {
+  // Note: this tests linker error ("undefined symbol"), the actual
+  // implementation is not important.
+  return nullptr;
+}
+
 //===----------------------------------------------------------------------===//
 // TestDialect
 //===----------------------------------------------------------------------===//
diff --git a/mlir/test/mlir-tblgen/attrdefs.td b/mlir/test/mlir-tblgen/attrdefs.td
index adec90dc5a371..d47411d6e860a 100644
--- a/mlir/test/mlir-tblgen/attrdefs.td
+++ b/mlir/test/mlir-tblgen/attrdefs.td
@@ -186,3 +186,16 @@ def I_TestGenMnemonicAliasAttr : TestAttr<"TestGenMnemonicAlias"> {
 // DEF-NEXT: os << "test_gen_mnemonic_alias";
 // DEF-NEXT: return ::mlir::OpAsmAliasResult::OverridableAlias;
 // DEF-NEXT: }
+
+def J_CustomStorageCtorAttr : AttrDef<Test_Dialect, "CustomStorageCtorAttr"> {
+  let attrName = "test_custom_storage_ctor_attr";
+  let parameters = (ins "bool":$flag);
+  let hasStorageCustomConstructor = 1;
+}
+
+// Note: ';' at the end of construct method declaration is important - otherwise
+// one cannot provide custom definition
+
+// DEF-LABEL: struct CustomStorageCtorAttrAttrStorage : public ::mlir::AttributeStorage
+// DEF: static CustomStorageCtorAttrAttrStorage *construct
+// DEF-SAME: (::mlir::AttributeStorageAllocator &allocator, KeyTy &&tblgenKey);
diff --git a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
index d9aa901ee2b28..dbae2143b920a 100644
--- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
+++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
@@ -668,10 +668,10 @@ void DefGen::emitHashKey() {
 }
 
 void DefGen::emitConstruct() {
-  Method *construct = storageCls->addMethod<Method::Inline>(
+  Method *construct = storageCls->addMethod(
       strfmt("{0} *", def.getStorageClassName()), "construct",
       def.hasStorageCustomConstructor() ? Method::StaticDeclaration
-                                        : Method::Static,
+                                        : Method::StaticInline,
       MethodParameter(strfmt("::mlir::{0}StorageAllocator &", valueType),
                       "allocator"),
       MethodParameter("KeyTy &&", "tblgenKey"));

>From 2ebae5ae9ae69415f9742f85848231c7dddbd91c Mon Sep 17 00:00:00 2001
From: "Golubev, Andrey" <andrey.golubev at intel.com>
Date: Thu, 10 Jul 2025 13:27:45 +0000
Subject: [PATCH 2/2] Add an extra test for Type

---
 mlir/test/lib/Dialect/Test/TestTypeDefs.td | 7 +++++++
 mlir/test/lib/Dialect/Test/TestTypes.cpp   | 8 ++++++++
 2 files changed, 15 insertions(+)

diff --git a/mlir/test/lib/Dialect/Test/TestTypeDefs.td b/mlir/test/lib/Dialect/Test/TestTypeDefs.td
index 03261f37c815d..ea20597231d58 100644
--- a/mlir/test/lib/Dialect/Test/TestTypeDefs.td
+++ b/mlir/test/lib/Dialect/Test/TestTypeDefs.td
@@ -352,6 +352,13 @@ def TestTypeCustomString : Test_Type<"TestTypeCustomString"> {
                               custom<BarString>(ref($foo)) `>` }];
 }
 
+def TestCustomStorageCtor : Test_Type<"TestCustomStorageCtor"> {
+    let mnemonic = "custom_storage_ctor_type";
+    let parameters = (ins "int":$value);
+    let assemblyFormat = "`<` $value `>`";
+    let hasStorageCustomConstructor = 1;
+}
+
 def TestTypeOptionalString : Test_Type<"TestTypeOptionalString"> {
   let parameters = (ins StringRefParameter<"description", [{"default"}]>:$str);
   let mnemonic = "optional_type_string";
diff --git a/mlir/test/lib/Dialect/Test/TestTypes.cpp b/mlir/test/lib/Dialect/Test/TestTypes.cpp
index 2fc2f90ef6bc0..bea043f56fe21 100644
--- a/mlir/test/lib/Dialect/Test/TestTypes.cpp
+++ b/mlir/test/lib/Dialect/Test/TestTypes.cpp
@@ -392,6 +392,14 @@ getCustomAssemblyFormatDynamicType(TestDialect *testDialect) {
                                     std::move(parser), std::move(printer));
 }
 
+test::detail::TestCustomStorageCtorTypeStorage *
+test::detail::TestCustomStorageCtorTypeStorage::construct(
+    mlir::StorageUniquer::StorageAllocator &, std::tuple<int> &&) {
+  // Note: this tests linker error ("undefined symbol"), the actual
+  // implementation is not important.
+  return nullptr;
+}
+
 //===----------------------------------------------------------------------===//
 // TestDialect
 //===----------------------------------------------------------------------===//