[Mlir-commits] [mlir] [MLIR] Add inherent attribute visitor (PR #217881)
Mehdi Amini
llvmlistbot at llvm.org
Fri Aug 21 04:00:00 PDT 2026
https://github.com/joker-eph created https://github.com/llvm/llvm-project/pull/217881
Visit inherent attributes directly from operation property storage.
Allow visitors to replace values through generated property conversions.
Keep populateInherentAttrs as a wrapper and migrate core IR callers.
This is intended as a more efficient than populateInherentAttrs that avoids materializing the list.
Assisted-by: Codex
>From 2a14e7587e6808b700303b016721785e7ac7c845 Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Fri, 21 Aug 2026 03:12:44 -0700
Subject: [PATCH] [MLIR] Add inherent attribute visitor
Visit inherent attributes directly from operation property storage.
Allow visitors to replace values through generated property conversions.
Keep populateInherentAttrs as a wrapper and migrate core IR callers.
This is intended as a more efficient than populateInherentAttrs that avoids
materializing the list.
Assisted-by: Codex
---
mlir/include/mlir/IR/ExtensibleDialect.h | 3 +-
mlir/include/mlir/IR/OperationSupport.h | 28 +++++++++----
mlir/lib/IR/AsmPrinter.cpp | 4 +-
mlir/lib/IR/AttrTypeSubElements.cpp | 11 ++++-
mlir/lib/IR/MLIRContext.cpp | 4 +-
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp | 46 ++++++++++++---------
mlir/unittests/IR/OpPropertiesTest.cpp | 5 +--
mlir/unittests/TableGen/OpBuildGen.cpp | 26 +++++++++++-
8 files changed, 91 insertions(+), 36 deletions(-)
diff --git a/mlir/include/mlir/IR/ExtensibleDialect.h b/mlir/include/mlir/IR/ExtensibleDialect.h
index 2219d6fd57977..79c07cbefcc5c 100644
--- a/mlir/include/mlir/IR/ExtensibleDialect.h
+++ b/mlir/include/mlir/IR/ExtensibleDialect.h
@@ -552,7 +552,8 @@ class DynamicOpDefinition : public OperationName::Impl {
void setInherentAttr(Operation *op, StringAttr name, Attribute value) final {
llvm::report_fatal_error("Unsupported setInherentAttr on Dynamic dialects");
}
- void populateInherentAttrs(Operation *op, NamedAttrList &attrs) final {}
+ void walkInherentAttrs(Operation *op,
+ OperationName::InherentAttrVisitor visitor) final {}
LogicalResult
verifyInherentAttrs(OperationName opName, NamedAttrList &attributes,
function_ref<InFlightDiagnostic()> emitError) final {
diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 5d8231c71469e..bfafc6cd77055 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -107,6 +107,7 @@ class OperationName {
// class is defined below.
using PopulateDefaultAttrsFn =
llvm::unique_function<void(const OperationName &, NamedAttrList &) const>;
+ using InherentAttrVisitor = llvm::function_ref<void(StringAttr, Attribute &)>;
using PrintAssemblyFn =
llvm::unique_function<void(Operation *, OpAsmPrinter &, StringRef) const>;
using VerifyInvariantsFn =
@@ -136,7 +137,8 @@ class OperationName {
StringRef name) = 0;
virtual void setInherentAttr(Operation *op, StringAttr name,
Attribute value) = 0;
- virtual void populateInherentAttrs(Operation *op, NamedAttrList &attrs) = 0;
+ virtual void walkInherentAttrs(Operation *op,
+ InherentAttrVisitor visitor) = 0;
virtual LogicalResult
verifyInherentAttrs(OperationName opName, NamedAttrList &attributes,
function_ref<InFlightDiagnostic()> emitError) = 0;
@@ -225,7 +227,7 @@ class OperationName {
std::optional<Attribute> getInherentAttr(Operation *op,
StringRef name) final;
void setInherentAttr(Operation *op, StringAttr name, Attribute value) final;
- void populateInherentAttrs(Operation *op, NamedAttrList &attrs) final;
+ void walkInherentAttrs(Operation *op, InherentAttrVisitor visitor) final;
LogicalResult
verifyInherentAttrs(OperationName opName, NamedAttrList &attributes,
function_ref<InFlightDiagnostic()> emitError) final;
@@ -416,9 +418,15 @@ class OperationName {
return getImpl()->setInherentAttr(op, name, value);
}
- void populateInherentAttrs(Operation *op, NamedAttrList &attrs) const {
- return getImpl()->populateInherentAttrs(op, attrs);
+ /// Visit the inherent attributes stored in the properties of `op`. The
+ /// visitor may replace an attribute by assigning to the attribute value.
+ void walkInherentAttrs(Operation *op, InherentAttrVisitor visitor) const {
+ getImpl()->walkInherentAttrs(op, visitor);
}
+
+ /// Append the inherent attributes stored in the properties of `op` to
+ /// `attrs`.
+ void populateInherentAttrs(Operation *op, NamedAttrList &attrs) const;
/// This method exists for backward compatibility purpose when using
/// properties to store inherent attributes, it enables validating the
/// attributes when parsed from the older generic syntax pre-Properties.
@@ -607,11 +615,11 @@ class RegisteredOperationName : public OperationName {
llvm_unreachable(
"Can't call setInherentAttr on operation with empty properties");
}
- void populateInherentAttrs(Operation *op, NamedAttrList &attrs) final {
+ void walkInherentAttrs(Operation *op, InherentAttrVisitor visitor) final {
if constexpr (hasProperties) {
auto concreteOp = cast<ConcreteOp>(op);
- ConcreteOp::populateInherentAttrs(concreteOp->getContext(),
- concreteOp.getProperties(), attrs);
+ ConcreteOp::walkInherentAttrs(concreteOp->getContext(),
+ concreteOp.getProperties(), visitor);
}
}
LogicalResult
@@ -955,6 +963,12 @@ class NamedAttrList {
mutable llvm::PointerIntPair<Attribute, 1, bool> dictionarySorted;
};
+inline void OperationName::populateInherentAttrs(Operation *op,
+ NamedAttrList &attrs) const {
+ walkInherentAttrs(
+ op, [&](StringAttr name, Attribute &attr) { attrs.append(name, attr); });
+}
+
//===----------------------------------------------------------------------===//
// OperationState
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index 47a33a116f92c..17eff9590220d 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/mlir/lib/IR/AsmPrinter.cpp
@@ -762,8 +762,10 @@ class DummyAliasOperationPrinter : private OpAsmPrinter {
printType(type);
// Consider the attributes of the operation for aliases.
- for (const NamedAttribute &attr : op->getAttrs())
+ for (const NamedAttribute &attr : op->getRawDictionaryAttrs())
printAttribute(attr.getValue());
+ op->getName().walkInherentAttrs(
+ op, [&](StringAttr, Attribute &attr) { printAttribute(attr); });
}
/// Print the given block. If 'printBlockArgs' is false, the arguments of the
diff --git a/mlir/lib/IR/AttrTypeSubElements.cpp b/mlir/lib/IR/AttrTypeSubElements.cpp
index 863e1aa95f2fb..45b8dd312a0f9 100644
--- a/mlir/lib/IR/AttrTypeSubElements.cpp
+++ b/mlir/lib/IR/AttrTypeSubElements.cpp
@@ -94,8 +94,15 @@ void detail::AttrTypeReplacerBase<Concrete>::replaceElementsIn(
// Update the attribute dictionary.
if (replaceAttrs) {
- if (auto newAttrs = replaceIfDifferent(op->getAttrDictionary()))
- op->setAttrs(cast<DictionaryAttr>(newAttrs));
+ if (auto newAttrs = replaceIfDifferent(op->getRawDictionaryAttrs()))
+ op->setDiscardableAttrs(cast<DictionaryAttr>(newAttrs));
+
+ if (op->getPropertiesStorageSize()) {
+ op->getName().walkInherentAttrs(op, [&](StringAttr, Attribute &attr) {
+ if (Attribute replacement = replaceIfDifferent(attr))
+ attr = replacement;
+ });
+ }
}
// If we aren't updating locations or types, we're done.
diff --git a/mlir/lib/IR/MLIRContext.cpp b/mlir/lib/IR/MLIRContext.cpp
index 06da14504a293..5efbe0e284147 100644
--- a/mlir/lib/IR/MLIRContext.cpp
+++ b/mlir/lib/IR/MLIRContext.cpp
@@ -1007,8 +1007,8 @@ void OperationName::UnregisteredOpModel::setInherentAttr(Operation *op,
*op->getPropertiesStorage().as<Attribute *>() =
attrs.getDictionary(op->getContext());
}
-void OperationName::UnregisteredOpModel::populateInherentAttrs(
- Operation *op, NamedAttrList &attrs) {}
+void OperationName::UnregisteredOpModel::walkInherentAttrs(
+ Operation *op, InherentAttrVisitor visitor) {}
LogicalResult OperationName::UnregisteredOpModel::verifyInherentAttrs(
OperationName opName, NamedAttrList &attributes,
function_ref<InFlightDiagnostic()> emitError) {
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index d7c119a31795a..573884c960d47 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -1404,12 +1404,15 @@ void OpEmitter::genPropertiesSupport() {
MethodParameter("llvm::StringRef", "name"),
MethodParameter("mlir::Attribute", "value"))
->body();
- auto &populateInherentAttrsMethod =
+ auto &walkInherentAttrsMethod =
opClass
- .addStaticMethod("void", "populateInherentAttrs",
+ .addStaticMethod("void", "walkInherentAttrs",
MethodParameter("::mlir::MLIRContext *", "ctx"),
- MethodParameter("const Properties &", "prop"),
- MethodParameter("::mlir::NamedAttrList &", "attrs"))
+ MethodParameter("Properties &", "prop"),
+ MethodParameter("::llvm::function_ref<void("
+ "::mlir::StringAttr, "
+ "::mlir::Attribute &)>",
+ "visitor"))
->body();
auto &verifyInherentAttrsMethod =
opClass
@@ -1630,8 +1633,14 @@ void OpEmitter::genPropertiesSupport() {
return;
}
)decl";
- const char *populateInherentAttrsMethodFmt = R"decl(
- if (prop.{0}) attrs.append("{0}", prop.{0});
+ const char *walkInherentAttrsMethodFmt = R"decl(
+ if (prop.{0}) {{
+ ::mlir::Attribute value = prop.{0};
+ ::mlir::Attribute originalValue = value;
+ visitor(::mlir::StringAttr::get(ctx, "{0}"), value);
+ if (value != originalValue)
+ setInherentAttr(prop, "{0}", value);
+ }
)decl";
for (const auto &attrOrProp : attrOrProperties) {
if (const auto *namedAttr =
@@ -1639,8 +1648,7 @@ void OpEmitter::genPropertiesSupport() {
StringRef name = namedAttr->attrName;
getInherentAttrMethod << formatv(getInherentAttrMethodFmt, name);
setInherentAttrMethod << formatv(setInherentAttrMethodFmt, name);
- populateInherentAttrsMethod
- << formatv(populateInherentAttrsMethodFmt, name);
+ walkInherentAttrsMethod << formatv(walkInherentAttrsMethodFmt, name);
continue;
}
// The ODS segment size property is "special": we expose it as an attribute
@@ -1689,17 +1697,17 @@ void OpEmitter::genPropertiesSupport() {
}
)decl",
name);
- if (name == operandSegmentAttrName) {
- populateInherentAttrsMethod << formatv(
- " attrs.append(\"{0}\", [&]() -> ::mlir::Attribute { {1} }());\n",
- operandSegmentAttrName,
- tgfmt(prop.getConvertToAttributeCall(), &fctx));
- } else {
- populateInherentAttrsMethod << formatv(
- " attrs.append(\"{0}\", [&]() -> ::mlir::Attribute { {1} }());\n",
- resultSegmentAttrName,
- tgfmt(prop.getConvertToAttributeCall(), &fctx));
- }
+ walkInherentAttrsMethod
+ << formatv(" {{\n"
+ " ::mlir::Attribute value = [&]() -> ::mlir::Attribute "
+ "{ {1} }();\n"
+ " ::mlir::Attribute originalValue = value;\n"
+ " visitor(::mlir::StringAttr::get(ctx, \"{0}\"), "
+ "value);\n"
+ " if (value != originalValue)\n"
+ " setInherentAttr(prop, \"{0}\", value);\n"
+ " }\n",
+ name, tgfmt(prop.getConvertToAttributeCall(), &fctx));
}
getInherentAttrMethod << " return std::nullopt;\n";
diff --git a/mlir/unittests/IR/OpPropertiesTest.cpp b/mlir/unittests/IR/OpPropertiesTest.cpp
index bea69e9e8f107..492b7a345e394 100644
--- a/mlir/unittests/IR/OpPropertiesTest.cpp
+++ b/mlir/unittests/IR/OpPropertiesTest.cpp
@@ -123,9 +123,8 @@ class OpWithProperties : public Op<OpWithProperties> {
}
static void setInherentAttr(Properties &prop, StringRef name,
mlir::Attribute value) {}
- static void populateInherentAttrs(MLIRContext *context,
- const Properties &prop,
- NamedAttrList &attrs) {}
+ static void walkInherentAttrs(MLIRContext *context, Properties &prop,
+ OperationName::InherentAttrVisitor visitor) {}
static LogicalResult
verifyInherentAttrs(OperationName opName, NamedAttrList &attrs,
function_ref<InFlightDiagnostic()> emitError) {
diff --git a/mlir/unittests/TableGen/OpBuildGen.cpp b/mlir/unittests/TableGen/OpBuildGen.cpp
index 09430336de994..6a6273fbf3384 100644
--- a/mlir/unittests/TableGen/OpBuildGen.cpp
+++ b/mlir/unittests/TableGen/OpBuildGen.cpp
@@ -288,6 +288,18 @@ TEST_F(OpBuildGenTest, BuildMethodsVariadicProperties) {
op = test::TableGenBuildOp6::create(builder, loc,
ValueRange{*cstI32, *cstI32}, attrs);
verifyOp(std::move(op), {f32Ty}, {*cstI32}, {*cstI32}, attrs);
+
+ // Test replacing an inherent attribute backed by a native property.
+ op = test::TableGenBuildOp6::create(builder, loc, f32Ty, ValueRange{*cstI32},
+ ValueRange{*cstI32});
+ DenseI32ArrayAttr replacement = builder.getDenseI32ArrayAttr({0, 2});
+ op->getName().walkInherentAttrs(op, [&](StringAttr name, Attribute &attr) {
+ if (name == "operandSegmentSizes")
+ attr = replacement;
+ });
+ EXPECT_EQ(op.getProperties().operandSegmentSizes[0], 0);
+ EXPECT_EQ(op.getProperties().operandSegmentSizes[1], 2);
+ op.erase();
}
TEST_F(OpBuildGenTest, BuildMethodsInherentDiscardableAttrs) {
@@ -296,7 +308,19 @@ TEST_F(OpBuildGenTest, BuildMethodsInherentDiscardableAttrs) {
ArrayRef<NamedAttribute> discardableAttrs = attrs.drop_front();
auto op7 = test::TableGenBuildOp7::create(
builder, loc, TypeRange{}, ValueRange{}, props, discardableAttrs);
- verifyOp(op7, {}, {}, attrs);
+ unsigned numInherentAttrs = 0;
+ BoolAttr replacement = builder.getBoolAttr(false);
+ op7->getName().walkInherentAttrs(op7, [&](StringAttr name, Attribute &attr) {
+ EXPECT_EQ(name, attrs[0].getName());
+ EXPECT_EQ(attr, attrs[0].getValue());
+ attr = replacement;
+ ++numInherentAttrs;
+ });
+ EXPECT_EQ(numInherentAttrs, 1u);
+ EXPECT_EQ(op7.getProperties().getAttr0(), replacement);
+ std::vector<NamedAttribute> replacedAttrs(attrs.begin(), attrs.end());
+ replacedAttrs[0].setValue(replacement);
+ verifyOp(op7, {}, {}, replacedAttrs);
// Check that the old-style builder where all the attributes go in the same
// place works.
More information about the Mlir-commits
mailing list