[Mlir-commits] [mlir] [mlir][ODS] Share generated legacy property splitting (NFC) (PR #226085)

[Mehdi Amini]

https://github.com/joker-eph updated https://github.com/llvm/llvm-project/pull/226085

>From 113c9238771a43dfac490348817dbc904a4bd84a Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Wed, 23 Sep 2026 17:03:41 -0700
Subject: [PATCH] [mlir][ODS] Share generated legacy property splitting

Route generated legacy builder attributes through an out-of-line helper that
partitions inherent and discardable attributes before converting properties.
This avoids compiling the same loop and error path for every operation.

Assisted-by: Codex
---
 mlir/include/mlir/IR/OperationSupport.h     |  8 ++++
 mlir/lib/IR/OperationSupport.cpp            | 20 ++++++++++
 mlir/test/mlir-tblgen/op-decl-and-defs.td   |  8 ++--
 mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp | 43 ++++++++-------------
 4 files changed, 48 insertions(+), 31 deletions(-)

diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 0a38631fdce03e..991d9847672e2e 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -69,6 +69,14 @@ namespace detail {
 void appendAttributeProperty(llvm::SmallVectorImpl<NamedAttribute> &attrs,
                              StringRef name, Attribute attr);
 
+/// Route legacy builder attributes to either the operation's properties or
+/// its discardable attribute dictionary. The callback handles conversion and
+/// diagnostics for the operation-specific properties.
+void splitPropertiesAndDiscardableAttributes(
+    OperationState &state, ArrayRef<NamedAttribute> attributes,
+    ArrayRef<StringRef> inherentNames,
+    llvm::function_ref<LogicalResult(DictionaryAttr)> setProperties);
+
 /// Assign a generated attribute-backed property after checking its type.
 /// Keep the conversion out of each operation's generated property setter.
 template <typename AttrT>
diff --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index 5b8eeb4f789a86..545791df641fef 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -16,6 +16,7 @@
 #include "mlir/IR/BuiltinTypes.h"
 #include "mlir/IR/OpDefinition.h"
 #include "mlir/IR/OpImplementation.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/SHA1.h"
 #include <numeric>
 #include <optional>
@@ -54,6 +55,25 @@ ParseResult mlir::detail::parseOptionalTypeInto(AsmParser &parser,
   return success();
 }
 
+void mlir::detail::splitPropertiesAndDiscardableAttributes(
+    OperationState &state, ArrayRef<NamedAttribute> attributes,
+    ArrayRef<StringRef> inherentNames,
+    llvm::function_ref<LogicalResult(DictionaryAttr)> setProperties) {
+  SmallVector<NamedAttribute> inherentAttributes;
+  for (const NamedAttribute &attr : attributes) {
+    StringRef name = attr.getName().getValue();
+    if (llvm::is_contained(inherentNames, name))
+      inherentAttributes.push_back(attr);
+    else
+      state.addAttribute(attr.getName(), attr.getValue());
+  }
+  if (inherentAttributes.empty())
+    return;
+  if (failed(setProperties(
+          DictionaryAttr::get(state.getContext(), inherentAttributes))))
+    llvm::report_fatal_error("Property conversion failed.");
+}
+
 //===----------------------------------------------------------------------===//
 // NamedAttrList
 //===----------------------------------------------------------------------===//
diff --git a/mlir/test/mlir-tblgen/op-decl-and-defs.td b/mlir/test/mlir-tblgen/op-decl-and-defs.td
index bcc81cd74d9768..36b8ebc78bf596 100644
--- a/mlir/test/mlir-tblgen/op-decl-and-defs.td
+++ b/mlir/test/mlir-tblgen/op-decl-and-defs.td
@@ -169,10 +169,10 @@ def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> {
 // DEFS-LABEL: void AOp::buildPropertiesAndDiscardableAttributes
 // DEFS:   Properties &properties = odsState.getOrAddProperties<Properties>();
 // DEFS-NEXT:   populateDefaultProperties(odsState.name, properties);
-// DEFS:     if (name == "attr1" || name == "some_attr2")
-// DEFS:       inherentAttributes.push_back(attr);
-// DEFS:       odsState.addAttribute(attr.getName(), attr.getValue());
-// DEFS:   if (::mlir::failed(setPropertiesFromAttr(
+// DEFS:   const ::llvm::StringRef inherentNames[] = {"attr1", "some_attr2"};
+// DEFS:   ::mlir::detail::splitPropertiesAndDiscardableAttributes(
+// DEFS:   return setPropertiesFromAttr(properties, inherentAttrs,
+
 
 // Check that `getAttrDictionary()` is used when not using properties.
 
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index f963d706301365..07070ab91387ce 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -2587,33 +2587,22 @@ void OpEmitter::genLegacyPropertiesBuilderHelper() {
   MethodBody &body = method->body();
   body << "  Properties &properties = " << builderOpStateProperties << ";\n"
        << "  populateDefaultProperties(" << builderOpState
-       << ".name, properties);\n"
-       << "  ::llvm::SmallVector<::mlir::NamedAttribute> "
-          "inherentAttributes;\n"
-       << "  for (const ::mlir::NamedAttribute &attr : attributes) {\n"
-       << "    ::llvm::StringRef name = attr.getName().getValue();\n"
-       << "    if (";
-  llvm::interleave(
-      inherentNames,
-      [&](StringRef name) { body << "name == \"" << name << "\""; },
-      [&] { body << " || "; });
-  body << ")\n"
-       << "      inherentAttributes.push_back(attr);\n"
-       << "    else\n"
-       << "      " << builderOpState << ".addAttribute(attr.getName(), "
-       << "attr.getValue());\n"
-       << "  }\n"
-       << "  if (inherentAttributes.empty())\n"
-       << "    return;\n";
-  // TODO: Use `PropertyKind<T>::unfreeze` to populate properties directly once
-  // it is available systematically, avoiding the DictionaryAttr conversion.
-  body << "  if (::mlir::failed(setPropertiesFromAttr(\n"
-       << "          properties,\n"
-       << "          ::mlir::DictionaryAttr::get(" << builderOpState
-       << ".getContext(), inherentAttributes),\n"
-       << "          [&]() { return ::mlir::emitError(" << builderOpState
-       << ".location); })))\n"
-       << "    ::llvm::report_fatal_error(\"Property conversion failed.\");\n";
+       << ".name, properties);\n";
+  if (!inherentNames.empty()) {
+    body << "  const ::llvm::StringRef inherentNames[] = {";
+    llvm::interleaveComma(inherentNames, body,
+                          [&](StringRef name) { body << '"' << name << '"'; });
+    body << "};\n";
+  }
+  body << "  ::mlir::detail::splitPropertiesAndDiscardableAttributes(\n"
+       << "      " << builderOpState << ", attributes, "
+       << (inherentNames.empty() ? "::llvm::ArrayRef<::llvm::StringRef>{}"
+                                 : "inherentNames")
+       << ", [&](::mlir::DictionaryAttr inherentAttrs) {\n"
+       << "        return setPropertiesFromAttr(properties, inherentAttrs,\n"
+       << "            [&]() { return ::mlir::emitError(" << builderOpState
+       << ".location); });\n"
+       << "      });\n";
 }
 
 void OpEmitter::genCodeForAddingPropertiesAndAttributes(