[Mlir-commits] [mlir] cc5f53a - [mlir][ODS] Reuse property storage in generated builders (NFC) (#226061)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Thu Sep 24 01:23:56 PDT 2026
Author: Mehdi Amini
Date: 2026-09-24T08:23:46Z
New Revision: cc5f53a2a7585b013f872b859f019d601a754307
URL: https://github.com/llvm/llvm-project/commit/cc5f53a2a7585b013f872b859f019d601a754307
DIFF: https://github.com/llvm/llvm-project/commit/cc5f53a2a7585b013f872b859f019d601a754307.diff
LOG: [mlir][ODS] Reuse property storage in generated builders (NFC) (#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
Added:
Modified:
mlir/test/mlir-tblgen/op-attribute.td
mlir/test/mlir-tblgen/op-decl-and-defs.td
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
Removed:
################################################################################
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
diff erent 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