[Mlir-commits] [mlir] [mlir][ODS] Reuse property storage in generated builders (NFC) (PR #226061)

[Mehdi Amini]

https://github.com/joker-eph created https://github.com/llvm/llvm-project/pull/226061

Retrieve operation properties once in each generated builder that writes them, then use the same reference for segment sizes, properties, and attributes. This improved MLIR build time by avoiding repeated storage lookup, e.g. cuts OpenMPDialect.cpp compiler instructions by 1.2%.

Assisted-by: Codex

>From 62cf03624237d05d188b766a8efa58f6f1630ee1 Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Wed, 23 Sep 2026 08:36:43 -0700
Subject: [PATCH] [mlir][ODS] Reuse property storage in generated builders

Retrieve operation properties once in each generated builder that writes
them, then use the same reference for segment sizes, properties, and
attributes. This avoids repeated storage lookup and cuts OpenMPDialect.cpp
compiler instructions by another 1.2%.

Assisted-by: Codex
(cherry picked from commit 2d498b36d0957183eed97e713f7a4402fc1d527e)
---
 mlir/test/mlir-tblgen/op-decl-and-defs.td   |  3 +++
 mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp | 24 ++++++++++++++-------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/mlir/test/mlir-tblgen/op-decl-and-defs.td b/mlir/test/mlir-tblgen/op-decl-and-defs.td
index 32c4bb88d188b..f9cb5695b982e 100644
--- a/mlir/test/mlir-tblgen/op-decl-and-defs.td
+++ b/mlir/test/mlir-tblgen/op-decl-and-defs.td
@@ -195,6 +195,9 @@ def NS_AttrSizedOperandOp : NS_Op<"attr_sized_operands",
 // CHECK:  RangeT getB() {
 // CHECK:  ValueT getC() {
 // CHECK:  RangeT getD() {
+// DEFS-LABEL: void AttrSizedOperandOp::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes,
+// DEFS-NEXT:   auto &odsProperties = odsState.getOrAddProperties<Properties>();
+// DEFS:   ::llvm::copy({{.*}}, odsProperties.operandSegmentSizes.begin());
 
 // Check op trait for different number of operands
 // ---
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index aa9ff159903bc..2fddead48132c 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -3265,6 +3265,17 @@ void OpEmitter::buildParamList(SmallVectorImpl<MethodParameter> &paramList,
 void OpEmitter::genCodeForAddingArgAndRegionForBuilder(
     MethodBody &body, llvm::StringSet<> &inferredAttributes,
     bool isRawValueAttr) {
+  bool needsProperties =
+      !op.getProperties().empty() ||
+      op.getTrait("::mlir::OpTrait::AttrSizedOperandSegments");
+  for (const NamedTypeConstraint &operand : op.getOperands())
+    needsProperties |= operand.isVariadicOfVariadic();
+  for (const NamedAttribute &attr : op.getAttributes())
+    needsProperties |=
+        !attr.attr.isDerivedAttr() && !inferredAttributes.contains(attr.name);
+  if (needsProperties)
+    body << "  auto &odsProperties = " << builderOpStateProperties << ";\n";
+
   // Push all operands to the result.
   for (int i = 0, e = op.getNumOperands(); i < e; ++i) {
     std::string argName = getArgumentName(op, i);
@@ -3280,7 +3291,7 @@ void OpEmitter::genCodeForAddingArgAndRegionForBuilder(
            << "      rangeSegments.push_back(range.size());\n"
            << "    auto rangeAttr = " << odsBuilder
            << ".getDenseI32ArrayAttr(rangeSegments);\n";
-      body << "    " << builderOpStateProperties << "."
+      body << "    odsProperties."
            << operand.constraint.getVariadicOfVariadicSegmentSizeAttr()
            << " = rangeAttr;";
       body << "  }\n";
@@ -3318,7 +3329,7 @@ void OpEmitter::genCodeForAddingArgAndRegionForBuilder(
   if (op.getTrait("::mlir::OpTrait::AttrSizedOperandSegments")) {
     body << "  ::llvm::copy(::llvm::ArrayRef<int32_t>({";
     emitSegment();
-    body << "}), " << builderOpStateProperties
+    body << "}), odsProperties"
          << ".operandSegmentSizes.begin());\n";
   }
 
@@ -3328,8 +3339,7 @@ void OpEmitter::genCodeForAddingArgAndRegionForBuilder(
     // interface type (used in the builder argument) to the storage type (used
     // in the state) is not necessarily trivial.
     std::string setterName = op.getSetterName(namedProp.name);
-    body << formatv("  {0}.{1}({2});\n", builderOpStateProperties, setterName,
-                    namedProp.name);
+    body << formatv("  odsProperties.{0}({1});\n", setterName, namedProp.name);
   }
   // Push all attributes to the result.
   for (const auto &namedAttr : op.getAttributes()) {
@@ -3353,12 +3363,10 @@ void OpEmitter::genCodeForAddingArgAndRegionForBuilder(
       // instance.
       FmtContext fctx;
       fctx.withBuilder("odsBuilder");
-      body << formatv("  {0}.{1} = {2};\n", builderOpStateProperties,
-                      namedAttr.name,
+      body << formatv("  odsProperties.{0} = {1};\n", namedAttr.name,
                       constBuildAttrFromParam(attr, fctx, namedAttr.name));
     } else {
-      body << formatv("  {0}.{1} = {1};\n", builderOpStateProperties,
-                      namedAttr.name);
+      body << formatv("  odsProperties.{0} = {0};\n", namedAttr.name);
     }
     if (emitNotNullCheck)
       body.unindent() << "  }\n";