[Mlir-commits] [mlir] [mlir][ODS] Reuse property storage in generated builders (NFC) (PR #226061)
Mehdi Amini
llvmlistbot at llvm.org
Thu Sep 24 01:08:10 PDT 2026
https://github.com/joker-eph updated https://github.com/llvm/llvm-project/pull/226061
>From f428db2183cd2d8135d8ef3be6cc2a5fbed80002 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-attribute.td | 30 +++++++++++----------
mlir/test/mlir-tblgen/op-decl-and-defs.td | 3 +++
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp | 24 +++++++++++------
3 files changed, 35 insertions(+), 22 deletions(-)
diff --git a/mlir/test/mlir-tblgen/op-attribute.td b/mlir/test/mlir-tblgen/op-attribute.td
index ebba86d32d82f..83a9e0ef79113 100644
--- a/mlir/test/mlir-tblgen/op-attribute.td
+++ b/mlir/test/mlir-tblgen/op-attribute.td
@@ -144,22 +144,23 @@ def AOp : NS_Op<"a_op", []> {
// ---
// DEF: void AOp::build(
-// DEF: odsState.getOrAddProperties<Properties>().aAttr = aAttr;
-// DEF: odsState.getOrAddProperties<Properties>().bAttr = bAttr;
+// DEF: auto &odsProperties = odsState.getOrAddProperties<Properties>();
+// DEF: odsProperties.aAttr = aAttr;
+// DEF: odsProperties.bAttr = bAttr;
// DEF: if (cAttr) {
-// DEF-NEXT: odsState.getOrAddProperties<Properties>().cAttr = cAttr;
+// DEF-NEXT: odsProperties.cAttr = cAttr;
-// DEF: odsState.getOrAddProperties<Properties>().aAttr = some-const-builder-call(odsBuilder, aAttr);
-// DEF-NEXT: odsState.getOrAddProperties<Properties>().bAttr = some-const-builder-call(odsBuilder, bAttr);
+// DEF: odsProperties.aAttr = some-const-builder-call(odsBuilder, aAttr);
+// DEF-NEXT: odsProperties.bAttr = some-const-builder-call(odsBuilder, bAttr);
// DEF-NEXT: if (cAttr) {
-// DEF-NEXT: odsState.getOrAddProperties<Properties>().cAttr = cAttr;
+// DEF-NEXT: odsProperties.cAttr = cAttr;
// DEF-NEXT: }
// DEF-NOT: if (dAttr)
-// DEF: odsState.getOrAddProperties<Properties>().dAttr = some-const-builder-call(odsBuilder, dAttr);
+// DEF: odsProperties.dAttr = some-const-builder-call(odsBuilder, dAttr);
// DEF: void AOp::build(
// DEF: some-return-type aAttr, some-return-type bAttr, /*optional*/some-attr-kind cAttr
-// DEF: odsState.getOrAddProperties<Properties>().aAttr = some-const-builder-call(odsBuilder, aAttr);
+// DEF: odsProperties.aAttr = some-const-builder-call(odsBuilder, aAttr);
// DEF: void AOp::build(
// DEF: ::llvm::ArrayRef<::mlir::NamedAttribute> attributes
@@ -272,14 +273,15 @@ def AgetOp : Op<Test2_Dialect, "a_get_op", []> {
// ---
// DEF: void AgetOp::build(
-// DEF: odsState.getOrAddProperties<Properties>().aAttr = aAttr;
-// DEF: odsState.getOrAddProperties<Properties>().bAttr = bAttr;
+// DEF: auto &odsProperties = odsState.getOrAddProperties<Properties>();
+// DEF: odsProperties.aAttr = aAttr;
+// DEF: odsProperties.bAttr = bAttr;
// DEF: if (cAttr) {
-// DEF-NEXT: odsState.getOrAddProperties<Properties>().cAttr = cAttr;
+// DEF-NEXT: odsProperties.cAttr = cAttr;
// DEF: void AgetOp::build(
// DEF: some-return-type aAttr, /*optional*/some-return-type bAttr, /*optional*/some-attr-kind cAttr
-// DEF: odsState.getOrAddProperties<Properties>().aAttr = some-const-builder-call(odsBuilder, aAttr);
+// DEF: odsProperties.aAttr = some-const-builder-call(odsBuilder, aAttr);
// DEF: void AgetOp::build(
// DEF: ::llvm::ArrayRef<::mlir::NamedAttribute> attributes
@@ -407,8 +409,8 @@ def DOp : NS_Op<"d_op", []> {
// DECL: static void build({{.*}}, uint32_t i32_attr, ::llvm::APFloat f64_attr, ::llvm::StringRef str_attr, bool bool_attr, ::SomeI32Enum enum_attr, uint32_t dv_i32_attr, ::llvm::APFloat dv_f64_attr, ::llvm::StringRef dv_str_attr = "abc", bool dv_bool_attr = true, ::SomeI32Enum dv_enum_attr = ::SomeI32Enum::case5)
// DEF-LABEL: DOp definitions
-// DEF: odsState.getOrAddProperties<Properties>().str_attr = odsBuilder.getStringAttr(str_attr);
-// DEF: odsState.getOrAddProperties<Properties>().dv_str_attr = odsBuilder.getStringAttr(dv_str_attr);
+// DEF: odsProperties.str_attr = odsBuilder.getStringAttr(str_attr);
+// DEF: odsProperties.dv_str_attr = odsBuilder.getStringAttr(dv_str_attr);
// Test default dictionary attribute.
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> ¶mList,
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";
More information about the Mlir-commits
mailing list