[Mlir-commits] [mlir] [mlir][ODS] Share parser operand storage cleanup (NFC) (PR #226094)
Mehdi Amini
llvmlistbot at llvm.org
Thu Sep 24 03:00:20 PDT 2026
https://github.com/joker-eph created https://github.com/llvm/llvm-project/pull/226094
Group variable-length operand vectors in generated parsers and move their construction and cleanup into a shared storage template. This reduces repeated cleanup code in parsers with many optional operand groups.
Assisted-by: Codex
>From 555b9709756ff59a26c458d8937f3e40a3954425 Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Wed, 23 Sep 2026 17:37:52 -0700
Subject: [PATCH] [mlir][ODS] Share parser operand storage cleanup
Group variable-length operand vectors in generated parsers and move their
construction and cleanup into a shared storage template. This reduces
repeated cleanup code in parsers with many optional operand groups.
Assisted-by: Codex
---
mlir/include/mlir/IR/OpImplementation.h | 20 +++++++++++++
mlir/test/mlir-tblgen/op-format.td | 4 +++
mlir/tools/mlir-tblgen/OpFormatGen.cpp | 40 +++++++++++++++++++------
3 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h
index 66fb5a31a07d6..e43cb98b89253 100644
--- a/mlir/include/mlir/IR/OpImplementation.h
+++ b/mlir/include/mlir/IR/OpImplementation.h
@@ -18,7 +18,9 @@
#include "mlir/IR/OpAsmSupport.h"
#include "mlir/IR/OpDefinition.h"
#include "llvm/ADT/Twine.h"
+#include "llvm/Support/Compiler.h"
#include "llvm/Support/SMLoc.h"
+#include <cstddef>
#include <optional>
namespace {
@@ -1831,6 +1833,24 @@ ParseResult parseOptionalOperandInto(
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &operands);
ParseResult parseOptionalTypeInto(AsmParser &parser,
SmallVectorImpl<Type> &types);
+
+/// Keep the cleanup of multiple generated parser operand groups out of each
+/// parser's early-return paths. The storage is shared across operations with
+/// the same number of groups.
+template <size_t N>
+class OperandParserStorage {
+public:
+ using Group = llvm::SmallVector<OpAsmParser::UnresolvedOperand, 4>;
+
+ LLVM_ATTRIBUTE_NOINLINE OperandParserStorage() {}
+ LLVM_ATTRIBUTE_NOINLINE ~OperandParserStorage() {}
+
+ Group &operator[](size_t index) { return groups[index]; }
+
+private:
+ Group groups[N];
+};
+
} // namespace detail
//===--------------------------------------------------------------------===//
diff --git a/mlir/test/mlir-tblgen/op-format.td b/mlir/test/mlir-tblgen/op-format.td
index 54d6e3e6249b6..fe37237dab27b 100644
--- a/mlir/test/mlir-tblgen/op-format.td
+++ b/mlir/test/mlir-tblgen/op-format.td
@@ -401,6 +401,10 @@ def OptionalGroupD : TestFormat_Op<[{
// segment sizes can be inferred from what was actually parsed, so the
// `operandSegmentSizes` key is left completely untouched: it isn't read,
// validated, or marked as a used key at all.
+// CHECK-LABEL: PropDictSegmentSizesInferred::parse
+// CHECK: ::mlir::detail::OperandParserStorage<2> odsOperandVectors;
+// CHECK: auto &a1Operands = odsOperandVectors[0];
+// CHECK: auto &a2Operands = odsOperandVectors[1];
// CHECK-LABEL: PropDictSegmentSizesInferred::setPropertiesFromParsedAttr
// CHECK-NOT: operandSegmentSizes
// CHECK: for (::mlir::NamedAttribute attr : dict) {
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index 9bbb38328767a..81269c90aadb2 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -19,6 +19,7 @@
#include "llvm/ADT/Sequence.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallBitVector.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/Twine.h"
#include "llvm/ADT/TypeSwitch.h"
@@ -951,7 +952,8 @@ static void genLiteralParser(StringRef value, MethodBody &body) {
/// Generate the storage code required for parsing the given element.
static void genElementParserStorage(FormatElement *element, const Operator &op,
- MethodBody &body) {
+ MethodBody &body,
+ bool groupOperandStorage) {
if (auto *optional = dyn_cast<OptionalElement>(element)) {
ArrayRef<FormatElement *> elements = optional->getThenElements();
@@ -963,20 +965,20 @@ static void genElementParserStorage(FormatElement *element, const Operator &op,
elidedAnchorElement = anchor;
for (FormatElement *childElement : elements)
if (childElement != elidedAnchorElement)
- genElementParserStorage(childElement, op, body);
+ genElementParserStorage(childElement, op, body, groupOperandStorage);
for (FormatElement *childElement : optional->getElseElements())
- genElementParserStorage(childElement, op, body);
+ genElementParserStorage(childElement, op, body, groupOperandStorage);
} else if (auto *oilist = dyn_cast<OIListElement>(element)) {
for (ArrayRef<FormatElement *> pelement : oilist->getParsingElements()) {
if (!oilist->getUnitVariableParsingElement(pelement))
for (FormatElement *element : pelement)
- genElementParserStorage(element, op, body);
+ genElementParserStorage(element, op, body, groupOperandStorage);
}
} else if (auto *custom = dyn_cast<CustomDirective>(element)) {
for (FormatElement *paramElement : custom->getElements())
- genElementParserStorage(paramElement, op, body);
+ genElementParserStorage(paramElement, op, body, groupOperandStorage);
} else if (isa<OperandsDirective>(element)) {
body << " ::llvm::SmallVector<::mlir::OpAsmParser::UnresolvedOperand, 4> "
@@ -996,9 +998,21 @@ static void genElementParserStorage(FormatElement *element, const Operator &op,
} else if (auto *operand = dyn_cast<OperandVariable>(element)) {
StringRef name = operand->getVar()->name;
if (operand->getVar()->isVariableLength()) {
- body
- << " ::llvm::SmallVector<::mlir::OpAsmParser::UnresolvedOperand, 4> "
- << name << "Operands;\n";
+ if (groupOperandStorage) {
+ unsigned index = 0;
+ for (const NamedTypeConstraint &candidate : op.getOperands()) {
+ if (&candidate == operand->getVar())
+ break;
+ if (candidate.isVariableLength())
+ ++index;
+ }
+ body << " auto &" << name << "Operands = odsOperandVectors[" << index
+ << "];\n";
+ } else {
+ body
+ << " ::llvm::SmallVector<::mlir::OpAsmParser::UnresolvedOperand, 4> "
+ << name << "Operands;\n";
+ }
if (operand->getVar()->isVariadicOfVariadic()) {
body << " llvm::SmallVector<int32_t> " << name
<< "OperandGroupSizes;\n";
@@ -1829,8 +1843,16 @@ void OperationFormat::genParser(Operator &op, OpClass &opClass) {
// Generate variables to store the operands and type within the format. This
// allows for referencing these variables in the presence of optional
// groupings.
+ unsigned numVariableLengthOperands = llvm::count_if(
+ op.getOperands(), [](const NamedTypeConstraint &operand) {
+ return operand.isVariableLength();
+ });
+ bool groupOperandStorage = !allOperands && numVariableLengthOperands > 1;
+ if (groupOperandStorage)
+ body << " ::mlir::detail::OperandParserStorage<"
+ << numVariableLengthOperands << "> odsOperandVectors;\n";
for (FormatElement *element : elements)
- genElementParserStorage(element, op, body);
+ genElementParserStorage(element, op, body, groupOperandStorage);
// A format context used when parsing attributes with buildable types.
FmtContext attrTypeCtx;
More information about the Mlir-commits
mailing list