[Mlir-commits] [mlir] [mlir][ODS] Remove unconditional properties query (NFC) (PR #221844)
Mehdi Amini
llvmlistbot at llvm.org
Mon Sep 7 15:03:27 PDT 2026
https://github.com/joker-eph created https://github.com/llvm/llvm-project/pull/221844
All operations emit a Properties struct, including operations without explicit properties. Remove the stale hasProperties query and fold its unreachable property-less paths from the operation, adaptor, and format generators.
Found by Coverity.
Assisted-by: Codex
>From a9a7df9b6d70161d855fe691f75a47d8217453b0 Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Mon, 7 Sep 2026 06:58:54 -0700
Subject: [PATCH] [mlir][ODS] Remove unconditional properties query
All operations emit a Properties struct, including operations without explicit
properties. Remove the stale hasProperties query and fold its unreachable
property-less paths from the operation, adaptor, and format generators.
Found by Coverity.
Assisted-by: Codex
---
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp | 389 ++++----------------
mlir/tools/mlir-tblgen/OpFormatGen.cpp | 107 ++----
mlir/tools/mlir-tblgen/OpFormatGen.h | 3 +-
3 files changed, 115 insertions(+), 384 deletions(-)
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 5688ab1f4691f..8becd36353cdf 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -76,18 +76,6 @@ static constexpr StringLiteral legacyOperandSegmentAttrName =
static constexpr StringLiteral legacyResultSegmentAttrName =
"result_segment_sizes";
-/// Code for an Op to lookup an attribute. Uses cached identifiers and subrange
-/// lookup.
-///
-/// {0}: Code snippet to get the attribute's name or identifier.
-/// {1}: The lower bound on the sorted subrange.
-/// {2}: The upper bound on the sorted subrange.
-/// {3}: Code snippet to get the array of named attributes.
-/// {4}: "Named" to get the named attribute.
-static const char *const subrangeGetAttr =
- "::mlir::impl::get{4}AttrFromSortedRange({3}.begin() + {1}, {3}.end() - "
- "{2}, {0})";
-
/// The logic to calculate the actual value range for a declared operand/result
/// of an op with variadic operands/results. Note that this logic is not for
/// general use; it assumes all variadic operands/results must have the same
@@ -302,11 +290,7 @@ static std::string constBuildAttrFromParam(const tblgen::Attribute &attr,
}
namespace {
-/// Metadata on a registered attribute. Given that attributes are stored in
-/// sorted order on operations, we can use information from ODS to deduce the
-/// number of required attributes less and and greater than each attribute,
-/// allowing us to search only a subrange of the attributes in ODS-generated
-/// getters.
+/// Metadata required to emit a registered or implicit attribute.
struct AttributeMetadata {
/// The attribute name.
StringRef attrName;
@@ -314,10 +298,6 @@ struct AttributeMetadata {
bool isRequired;
/// The ODS attribute constraint. Not present for implicit attributes.
std::optional<Attribute> constraint;
- /// The number of required attributes less than this attribute.
- unsigned lowerBound = 0;
- /// The number of required attributes greater than this attribute.
- unsigned upperBound = 0;
};
/// Helper class to select between OpAdaptor and Op code templates.
@@ -351,35 +331,13 @@ class OpOrAdaptorHelper {
};
// Generate code for getting an attribute.
- Formatter getAttr(StringRef attrName, bool isNamed = false) const {
+ Formatter getAttr(StringRef attrName) const {
assert(attrMetadata.count(attrName) && "expected attribute metadata");
- return [this, attrName, isNamed](raw_ostream &os) -> raw_ostream & {
- const AttributeMetadata &attr = attrMetadata.find(attrName)->second;
- if (hasProperties()) {
- assert(!isNamed);
- return os << "getProperties()." << attrName;
- }
- return os << formatv(subrangeGetAttr, getAttrName(attrName),
- attr.lowerBound, attr.upperBound, getAttrRange(),
- isNamed ? "Named" : "");
+ return [attrName](raw_ostream &os) -> raw_ostream & {
+ return os << "getProperties()." << attrName;
};
}
- // Generate code for getting the name of an attribute.
- Formatter getAttrName(StringRef attrName) const {
- return [this, attrName](raw_ostream &os) -> raw_ostream & {
- if (emitForOp)
- return os << op.getGetterName(attrName) << "AttrName()";
- return os << formatv("{0}::{1}AttrName(*odsOpName)", op.getCppClassName(),
- op.getGetterName(attrName));
- };
- }
-
- // Get the code snippet for getting the named attribute range.
- StringRef getAttrRange() const {
- return emitForOp ? "(*this)->getRawDictionaryAttrs()" : "odsAttrs";
- }
-
// Get the prefix code for emitting an error.
Formatter emitErrorPrefix() const {
return [this](raw_ostream &os) -> raw_ostream & {
@@ -422,19 +380,10 @@ class OpOrAdaptorHelper {
return attrMetadata;
}
- /// Returns whether to emit a `Properties` struct for this operation or not.
- bool hasProperties() const {
- if (!op.getProperties().empty())
- return true;
- return true;
- }
-
/// Returns whether the operation will have a non-empty `Properties` struct.
bool hasNonEmptyPropertiesStruct() const {
if (!op.getProperties().empty())
return true;
- if (!hasProperties())
- return false;
if (op.getTrait("::mlir::OpTrait::AttrSizedOperandSegments") ||
op.getTrait("::mlir::OpTrait::AttrSizedResultSegments"))
return true;
@@ -486,9 +435,6 @@ class OpOrAdaptorHelper {
// bytecode encodings, i.e. versions less than 6.
uint32_t operandSegmentSizesLegacyIndex = 0;
uint32_t resultSegmentSizesLegacyIndex = 0;
-
- // The number of required attributes.
- unsigned numRequired;
};
} // namespace
@@ -591,15 +537,6 @@ void OpOrAdaptorHelper::computeAttrMetadata() {
++resultSegmentSizesLegacyIndex;
}
- // Compute the subrange bounds for each attribute.
- numRequired = 0;
- for (AttributeMetadata &attr : sortedAttrMetadata) {
- attr.lowerBound = numRequired;
- numRequired += attr.isRequired;
- };
- for (AttributeMetadata &attr : sortedAttrMetadata)
- attr.upperBound = numRequired - attr.lowerBound - attr.isRequired;
-
// Store the results back into the map.
for (const AttributeMetadata &attr : sortedAttrMetadata)
attrMetadata.insert({attr.attrName, attr});
@@ -896,18 +833,15 @@ static bool canEmitAttrVerifier(Attribute attr, bool isEmittingForOp) {
//
// Attribute verification is performed as follows:
//
-// 1. Verify that all required attributes are present in sorted order. This
-// ensures that we can use subrange lookup even with potentially missing
-// attributes.
+// 1. Load inherent attributes from the Properties struct and verify that all
+// required attributes are present.
// 2. Verify native trait attributes so that other attributes may call methods
// that depend on the validity of these attributes, e.g. segment size attributes
// and operand or result getters.
// 3. Verify the constraints on all present attributes.
-static void
-genAttributeVerifier(const OpOrAdaptorHelper &emitHelper, FmtContext &ctx,
- MethodBody &body,
- const StaticVerifierFunctionEmitter &staticVerifierEmitter,
- bool useProperties) {
+static void genAttributeVerifier(
+ const OpOrAdaptorHelper &emitHelper, FmtContext &ctx, MethodBody &body,
+ const StaticVerifierFunctionEmitter &staticVerifierEmitter) {
if (emitHelper.getAttrMetadata().empty())
return;
@@ -936,36 +870,6 @@ genAttributeVerifier(const OpOrAdaptorHelper &emitHelper, FmtContext &ctx,
return ::mlir::failure();
)";
- // Traverse the array until the required attribute is found. Return an error
- // if the traversal reached the end.
- //
- // {0}: Code to get the name of the attribute.
- // {1}: The emit error prefix.
- // {2}: The name of the attribute.
- const char *const findRequiredAttr = R"(
-while (true) {{
- if (namedAttrIt == namedAttrRange.end())
- return {1}requires attribute '{2}'");
- if (namedAttrIt->getName() == {0}) {{
- tblgen_{2} = namedAttrIt->getValue();
- break;
- })";
-
- // Emit a check to see if the iteration has encountered an optional attribute.
- //
- // {0}: Code to get the name of the attribute.
- // {1}: The name of the attribute.
- const char *const checkOptionalAttr = R"(
- else if (namedAttrIt->getName() == {0}) {{
- tblgen_{1} = namedAttrIt->getValue();
- })";
-
- // Emit the start of the loop for checking trailing attributes.
- const char *const checkTrailingAttrs = R"(while (true) {
- if (namedAttrIt == namedAttrRange.end()) {
- break;
- })";
-
// Emit the verifier for the attribute.
const auto emitVerifier = [&](Attribute attr, StringRef attrName,
StringRef varName) {
@@ -990,65 +894,17 @@ while (true) {{
};
body.indent();
- if (useProperties) {
- for (const std::pair<StringRef, AttributeMetadata> &it :
- emitHelper.getAttrMetadata()) {
- const AttributeMetadata &metadata = it.second;
- if (metadata.constraint && metadata.constraint->isDerivedAttr())
- continue;
+ for (const std::pair<StringRef, AttributeMetadata> &it :
+ emitHelper.getAttrMetadata()) {
+ const AttributeMetadata &metadata = it.second;
+ if (metadata.constraint && metadata.constraint->isDerivedAttr())
+ continue;
+ body << formatv(
+ "auto tblgen_{0} = getProperties().{0}; (void)tblgen_{0};\n", it.first);
+ if (metadata.isRequired)
body << formatv(
- "auto tblgen_{0} = getProperties().{0}; (void)tblgen_{0};\n",
- it.first);
- if (metadata.isRequired)
- body << formatv(
- "if (!tblgen_{0}) return {1}requires attribute '{0}'\");\n",
- it.first, emitHelper.emitErrorPrefix());
- }
- } else {
- body << formatv("auto namedAttrRange = {0};\n", emitHelper.getAttrRange());
- body << "auto namedAttrIt = namedAttrRange.begin();\n";
-
- // Iterate over the attributes in sorted order. Keep track of the optional
- // attributes that may be encountered along the way.
- SmallVector<const AttributeMetadata *> optionalAttrs;
-
- for (const std::pair<StringRef, AttributeMetadata> &it :
- emitHelper.getAttrMetadata()) {
- const AttributeMetadata &metadata = it.second;
- if (!metadata.isRequired) {
- optionalAttrs.push_back(&metadata);
- continue;
- }
-
- body << formatv("::mlir::Attribute {0};\n", getVarName(it.first));
- for (const AttributeMetadata *optional : optionalAttrs) {
- body << formatv("::mlir::Attribute {0};\n",
- getVarName(optional->attrName));
- }
- body << formatv(findRequiredAttr, emitHelper.getAttrName(it.first),
- emitHelper.emitErrorPrefix(), it.first);
- for (const AttributeMetadata *optional : optionalAttrs) {
- body << formatv(checkOptionalAttr,
- emitHelper.getAttrName(optional->attrName),
- optional->attrName);
- }
- body << "\n ++namedAttrIt;\n}\n";
- optionalAttrs.clear();
- }
- // Get trailing optional attributes.
- if (!optionalAttrs.empty()) {
- for (const AttributeMetadata *optional : optionalAttrs) {
- body << formatv("::mlir::Attribute {0};\n",
- getVarName(optional->attrName));
- }
- body << checkTrailingAttrs;
- for (const AttributeMetadata *optional : optionalAttrs) {
- body << formatv(checkOptionalAttr,
- emitHelper.getAttrName(optional->attrName),
- optional->attrName);
- }
- body << "\n ++namedAttrIt;\n}\n";
- }
+ "if (!tblgen_{0}) return {1}requires attribute '{0}'\");\n", it.first,
+ emitHelper.emitErrorPrefix());
}
body.unindent();
@@ -1205,7 +1061,7 @@ OpEmitter::OpEmitter(const Operator &op,
genFolderDecls();
genTypeInterfaceMethods();
genOpInterfaceMethods();
- generateOpFormat(op, opClass, emitHelper.hasProperties());
+ generateOpFormat(op, opClass);
genSideEffectInterfaceMethods();
}
void OpEmitter::emitDecl(
@@ -1407,9 +1263,6 @@ SmallVector<OpEmitter::ConstArgument> OpEmitter::getAttrOrProperties() {
}
void OpEmitter::genPropertiesSupport() {
- if (!emitHelper.hasProperties())
- return;
-
SmallVector<ConstArgument> attrOrProperties = getAttrOrProperties();
auto &setPropMethod =
opClass
@@ -2380,18 +2233,12 @@ void OpEmitter::genNamedOperandSetters() {
body << " auto mutableRange = "
"::mlir::MutableOperandRange(getOperation(), "
"range.first, range.second";
- if (attrSizedOperands) {
- if (emitHelper.hasProperties())
- body << formatv(", ::mlir::MutableOperandRange::OperandSegment({0}u, "
- "{{getOperandSegmentSizesAttrName(), "
- "::mlir::DenseI32ArrayAttr::get(getContext(), "
- "getProperties().operandSegmentSizes)})",
- i);
- else
- body << formatv(
- ", ::mlir::MutableOperandRange::OperandSegment({0}u, *{1})", i,
- emitHelper.getAttr(operandSegmentAttrName, /*isNamed=*/true));
- }
+ if (attrSizedOperands)
+ body << formatv(", ::mlir::MutableOperandRange::OperandSegment({0}u, "
+ "{{getOperandSegmentSizesAttrName(), "
+ "::mlir::DenseI32ArrayAttr::get(getContext(), "
+ "getProperties().operandSegmentSizes)})",
+ i);
body << ");\n";
// If this operand is a nested variadic, we split the range into a
@@ -2867,63 +2714,27 @@ void OpEmitter::genPopulateDefaultAttributes() {
}))
return;
- if (emitHelper.hasProperties()) {
- SmallVector<MethodParameter> paramList;
- paramList.emplace_back("::mlir::OperationName", "opName");
- paramList.emplace_back("Properties &", "properties");
- auto *m =
- opClass.addStaticMethod("void", "populateDefaultProperties", paramList);
- ERROR_IF_PRUNED(m, "populateDefaultProperties", op);
- auto &body = m->body();
- body.indent();
- body << "::mlir::Builder " << odsBuilder << "(opName.getContext());\n";
- for (const NamedAttribute &namedAttr : op.getAttributes()) {
- auto &attr = namedAttr.attr;
- if (!attr.hasDefaultValue() || attr.isOptional())
- continue;
- StringRef name = namedAttr.name;
- FmtContext fctx;
- fctx.withBuilder(odsBuilder);
- body << "if (!properties." << name << ")\n"
- << " properties." << name << " = "
- << std::string(tgfmt(attr.getConstBuilderTemplate(), &fctx,
- tgfmt(attr.getDefaultValue(), &fctx)))
- << ";\n";
- }
- return;
- }
-
SmallVector<MethodParameter> paramList;
- paramList.emplace_back("const ::mlir::OperationName &", "opName");
- paramList.emplace_back("::mlir::NamedAttrList &", "attributes");
- auto *m = opClass.addStaticMethod("void", "populateDefaultAttrs", paramList);
- ERROR_IF_PRUNED(m, "populateDefaultAttrs", op);
+ paramList.emplace_back("::mlir::OperationName", "opName");
+ paramList.emplace_back("Properties &", "properties");
+ auto *m =
+ opClass.addStaticMethod("void", "populateDefaultProperties", paramList);
+ ERROR_IF_PRUNED(m, "populateDefaultProperties", op);
auto &body = m->body();
body.indent();
-
- // Set default attributes that are unset.
- body << "auto attrNames = opName.getAttributeNames();\n";
- body << "::mlir::Builder " << odsBuilder
- << "(attrNames.front().getContext());\n";
- StringMap<int> attrIndex;
- for (const auto &it : llvm::enumerate(emitHelper.getAttrMetadata())) {
- attrIndex[it.value().first] = it.index();
- }
+ body << "::mlir::Builder " << odsBuilder << "(opName.getContext());\n";
for (const NamedAttribute &namedAttr : op.getAttributes()) {
auto &attr = namedAttr.attr;
if (!attr.hasDefaultValue() || attr.isOptional())
continue;
- auto index = attrIndex[namedAttr.name];
- body << "if (!attributes.get(attrNames[" << index << "])) {\n";
+ StringRef name = namedAttr.name;
FmtContext fctx;
fctx.withBuilder(odsBuilder);
-
- std::string defaultValue =
- std::string(tgfmt(attr.getConstBuilderTemplate(), &fctx,
- tgfmt(attr.getDefaultValue(), &fctx)));
- body.indent() << formatv("attributes.append(attrNames[{0}], {1});\n", index,
- defaultValue);
- body.unindent() << "}\n";
+ body << "if (!properties." << name << ")\n"
+ << " properties." << name << " = "
+ << std::string(tgfmt(attr.getConstBuilderTemplate(), &fctx,
+ tgfmt(attr.getDefaultValue(), &fctx)))
+ << ";\n";
}
}
@@ -3167,8 +2978,7 @@ void OpEmitter::genBuilder() {
// 2. one having an aggregated parameter for all result types / operands /
// [properties / discardable] attributes, and
genCollectiveParamBuilder(CollectiveBuilderKind::AttrDict);
- if (emitHelper.hasProperties())
- genCollectiveParamBuilder(CollectiveBuilderKind::PropStruct);
+ genCollectiveParamBuilder(CollectiveBuilderKind::PropStruct);
// 3. one having a stand-alone parameter for each operand and attribute,
// use the first operand or attribute's type as all result types
// to facilitate different call patterns.
@@ -3177,9 +2987,8 @@ void OpEmitter::genBuilder() {
genUseOperandAsResultTypeSeparateParamBuilder();
genUseOperandAsResultTypeCollectiveParamBuilder(
CollectiveBuilderKind::AttrDict);
- if (emitHelper.hasProperties())
- genUseOperandAsResultTypeCollectiveParamBuilder(
- CollectiveBuilderKind::PropStruct);
+ genUseOperandAsResultTypeCollectiveParamBuilder(
+ CollectiveBuilderKind::PropStruct);
}
if (op.getTrait("::mlir::OpTrait::FirstAttrDerivedResultType")) {
genUseAttrAsResultTypeCollectiveParamBuilder(
@@ -3958,12 +3767,9 @@ void OpEmitter::genVerifier() {
opClass.addMethod("::llvm::LogicalResult", "verifyInvariantsImpl");
ERROR_IF_PRUNED(implMethod, "verifyInvariantsImpl", op);
auto &implBody = implMethod->body();
- bool useProperties = emitHelper.hasProperties();
-
populateSubstitutions(emitHelper, verifyCtx);
genPropertyVerifier(emitHelper, verifyCtx, implBody, staticVerifierEmitter);
- genAttributeVerifier(emitHelper, verifyCtx, implBody, staticVerifierEmitter,
- useProperties);
+ genAttributeVerifier(emitHelper, verifyCtx, implBody, staticVerifierEmitter);
genOperandResultVerifier(implBody, op.getOperands(), "operand");
genOperandResultVerifier(implBody, op.getResults(), "result");
@@ -4379,8 +4185,7 @@ OpOperandAdaptorEmitter::OpOperandAdaptorEmitter(
fctx.withBuilder(odsBuilder);
genericAdaptorBase.declare<VisibilityDeclaration>(Visibility::Public);
- bool useProperties = emitHelper.hasProperties();
- if (useProperties) {
+ {
// Define the properties struct with multiple members.
using ConstArgument =
llvm::PointerUnion<const AttributeMetadata *, const NamedProperty *>;
@@ -4497,12 +4302,12 @@ OpOperandAdaptorEmitter::OpOperandAdaptorEmitter(
genericAdaptorBase.declare<ExtraClassDeclaration>(
std::move(declarations));
}
+
genericAdaptorBase.declare<VisibilityDeclaration>(Visibility::Protected);
genericAdaptorBase.declare<Field>("::mlir::DictionaryAttr", "odsAttrs");
genericAdaptorBase.declare<Field>("::std::optional<::mlir::OperationName>",
"odsOpName");
- if (useProperties)
- genericAdaptorBase.declare<Field>("Properties", "properties");
+ genericAdaptorBase.declare<Field>("Properties", "properties");
genericAdaptorBase.declare<Field>("::mlir::RegionRange", "odsRegions");
genericAdaptor.addTemplateParam("RangeT");
@@ -4518,23 +4323,16 @@ OpOperandAdaptorEmitter::OpOperandAdaptorEmitter(
op.getTrait("::mlir::OpTrait::AttrSizedOperandSegments");
{
SmallVector<MethodParameter> paramList;
- if (useProperties) {
- // Properties can't be given a default constructor here due to Properties
- // struct being defined in the enclosing class which isn't complete by
- // here.
- paramList.emplace_back("::mlir::DictionaryAttr", "attrs");
- paramList.emplace_back("const Properties &", "properties");
- } else {
- paramList.emplace_back("::mlir::DictionaryAttr", "attrs", "{}");
- paramList.emplace_back("const ::mlir::EmptyProperties &", "properties",
- "{}");
- }
+ // Properties can't be given a default constructor here due to Properties
+ // struct being defined in the enclosing class which isn't complete by
+ // here.
+ paramList.emplace_back("::mlir::DictionaryAttr", "attrs");
+ paramList.emplace_back("const Properties &", "properties");
paramList.emplace_back("::mlir::RegionRange", "regions", "{}");
auto *baseConstructor =
genericAdaptorBase.addConstructor<Method::Inline>(paramList);
baseConstructor->addMemberInitializer("odsAttrs", "attrs");
- if (useProperties)
- baseConstructor->addMemberInitializer("properties", "properties");
+ baseConstructor->addMemberInitializer("properties", "properties");
baseConstructor->addMemberInitializer("odsRegions", "regions");
MethodBody &body = baseConstructor->body();
@@ -4555,37 +4353,25 @@ OpOperandAdaptorEmitter::OpOperandAdaptorEmitter(
paramList[2] = MethodParameter("::mlir::PropertyRef", "properties");
auto *propertyRefConstructor =
genericAdaptor.addConstructor(std::move(paramList));
- if (useProperties) {
- propertyRefConstructor->addMemberInitializer(
- genericAdaptor.getClassName(),
- "values, "
- "attrs, "
- "(properties ? *properties.as<Properties *>() : Properties{}), "
- "regions");
- } else {
- propertyRefConstructor->addMemberInitializer(
- genericAdaptor.getClassName(),
- "values, "
- "attrs, "
- "(properties ? *properties.as<::mlir::EmptyProperties *>() : "
- "::mlir::EmptyProperties{}), "
- "regions");
- }
+ propertyRefConstructor->addMemberInitializer(
+ genericAdaptor.getClassName(),
+ "values, "
+ "attrs, "
+ "(properties ? *properties.as<Properties *>() : Properties{}), "
+ "regions");
// Add forwarding constructor that constructs Properties.
- if (useProperties) {
- SmallVector<MethodParameter> paramList;
- paramList.emplace_back("RangeT", "values");
- paramList.emplace_back("::mlir::DictionaryAttr", "attrs",
- attrSizedOperands ? "" : "nullptr");
- auto *noPropertiesConstructor =
- genericAdaptor.addConstructor(std::move(paramList));
- noPropertiesConstructor->addMemberInitializer(
- genericAdaptor.getClassName(), "values, "
- "attrs, "
- "Properties{}, "
- "{}");
- }
+ SmallVector<MethodParameter> noPropertiesParamList;
+ noPropertiesParamList.emplace_back("RangeT", "values");
+ noPropertiesParamList.emplace_back("::mlir::DictionaryAttr", "attrs",
+ attrSizedOperands ? "" : "nullptr");
+ auto *noPropertiesConstructor =
+ genericAdaptor.addConstructor(std::move(noPropertiesParamList));
+ noPropertiesConstructor->addMemberInitializer(genericAdaptor.getClassName(),
+ "values, "
+ "attrs, "
+ "Properties{}, "
+ "{}");
}
// Create a constructor that creates a new generic adaptor by copying
@@ -4606,23 +4392,15 @@ OpOperandAdaptorEmitter::OpOperandAdaptorEmitter(
// and the value range from the parameter.
{
// Base class is in the cpp file and can simply access the members of the op
- // class to initialize the template independent fields. If the op doesn't
- // have properties, we can emit a generic constructor inline. Otherwise,
- // emit it out-of-line because we need the op to be defined.
- Constructor *constructor;
- if (useProperties) {
- constructor = genericAdaptorBase.addConstructor(
- MethodParameter(op.getCppClassName(), "op"));
- } else {
- constructor = genericAdaptorBase.addConstructor<Method::Inline>(
- MethodParameter("::mlir::Operation *", "op"));
- }
+ // class to initialize the template independent fields. Emit it out-of-line
+ // because we need the op to be defined to access its properties.
+ Constructor *constructor = genericAdaptorBase.addConstructor(
+ MethodParameter(op.getCppClassName(), "op"));
constructor->addMemberInitializer("odsAttrs",
"op->getRawDictionaryAttrs()");
// Retrieve the operation name from the op directly.
constructor->addMemberInitializer("odsOpName", "op->getName()");
- if (useProperties)
- constructor->addMemberInitializer("properties", "op.getProperties()");
+ constructor->addMemberInitializer("properties", "op.getProperties()");
constructor->addMemberInitializer("odsRegions", "op->getRegions()");
// Generic adaptor is templated and therefore defined inline in the header.
@@ -4668,13 +4446,10 @@ OpOperandAdaptorEmitter::OpOperandAdaptorEmitter(
// value, in which case the default value may be arbitrary code.
auto *method = genericAdaptorBase.addMethod(
attr.getStorageType(), emitName + "Attr",
- attr.hasDefaultValue() || !useProperties ? Method::Properties::None
- : Method::Properties::Inline);
+ attr.hasDefaultValue() ? Method::Properties::None
+ : Method::Properties::Inline);
ERROR_IF_PRUNED(method, "Adaptor::" + emitName + "Attr", op);
auto &body = method->body().indent();
- if (!useProperties)
- body << "assert(odsAttrs && \"no attributes when constructing "
- "adapter\");\n";
body << formatv(
"auto attr = ::llvm::{1}<{2}>({0});\n", emitHelper.getAttr(name),
attr.hasDefaultValue() || attr.isOptional() ? "dyn_cast_or_null"
@@ -4693,12 +4468,10 @@ OpOperandAdaptorEmitter::OpOperandAdaptorEmitter(
body << "return attr;\n";
};
- if (useProperties) {
- auto *m = genericAdaptorBase.addInlineMethod("const Properties &",
- "getProperties");
- ERROR_IF_PRUNED(m, "Adaptor::getProperties", op);
- m->body() << " return properties;";
- }
+ auto *m =
+ genericAdaptorBase.addInlineMethod("const Properties &", "getProperties");
+ ERROR_IF_PRUNED(m, "Adaptor::getProperties", op);
+ m->body() << " return properties;";
{
auto *m = genericAdaptorBase.addInlineMethod("::mlir::DictionaryAttr",
"getAttributes");
@@ -4774,13 +4547,11 @@ void OpOperandAdaptorEmitter::addVerification() {
MethodParameter("::mlir::Location", "loc"));
ERROR_IF_PRUNED(method, "verify", op);
auto &body = method->body();
- bool useProperties = emitHelper.hasProperties();
FmtContext verifyCtx;
populateSubstitutions(emitHelper, verifyCtx);
genPropertyVerifier(emitHelper, verifyCtx, body, staticVerifierEmitter);
- genAttributeVerifier(emitHelper, verifyCtx, body, staticVerifierEmitter,
- useProperties);
+ genAttributeVerifier(emitHelper, verifyCtx, body, staticVerifierEmitter);
body << " return ::mlir::success();";
}
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index 5b77e05214435..b4de01a99c75c 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -361,9 +361,8 @@ struct OperationFormat {
Optional
};
- OperationFormat(const Operator &op, bool hasProperties)
- : useProperties(hasProperties),
- useStrictPropertiesInAssemblyFormat(
+ OperationFormat(const Operator &op)
+ : useStrictPropertiesInAssemblyFormat(
op.getDialect().useStrictPropertiesInAssemblyFormat()),
opCppClassName(op.getCppClassName()) {
operandTypes.resize(op.getNumOperands(), TypeResolution());
@@ -425,9 +424,6 @@ struct OperationFormat {
/// A flag indicating if this operation has the SingleBlock trait.
bool hasSingleBlockTrait;
- /// Indicate whether we need to use properties for the current operator.
- bool useProperties;
-
/// Indicate whether the dialect uses strict properties in assembly formats.
bool useStrictPropertiesInAssemblyFormat;
@@ -1142,7 +1138,6 @@ static void genCustomParameterParser(FormatElement *param, MethodBody &body,
/// Generate the parser for a custom directive.
static void genCustomDirectiveParser(CustomDirective *dir, MethodBody &body,
- bool useProperties,
StringRef opCppClassName,
bool isOptional = false) {
body << " {\n";
@@ -1222,13 +1217,8 @@ static void genCustomDirectiveParser(CustomDirective *dir, MethodBody &body,
const NamedAttribute *var = attr->getVar();
if (var->attr.isOptional() || var->attr.hasDefaultValue())
body << formatv(" if ({0}Attr)\n ", var->name);
- if (useProperties) {
- std::string propAccess = formatv(getPropertiesCode, opCppClassName);
- body << formatv(" {0}.{1} = {1}Attr;\n", propAccess, var->name);
- } else {
- body << formatv(" result.addAttribute(\"{0}\", {0}Attr);\n",
- var->name);
- }
+ std::string propAccess = formatv(getPropertiesCode, opCppClassName);
+ body << formatv(" {0}.{1} = {1}Attr;\n", propAccess, var->name);
} else if (auto *operand = dyn_cast<OperandVariable>(param)) {
const NamedTypeConstraint *var = operand->getVar();
if (var->isOptional()) {
@@ -1265,7 +1255,7 @@ static void genCustomDirectiveParser(CustomDirective *dir, MethodBody &body,
/// Generate the parser for a enum attribute.
static void genEnumAttrParser(const NamedAttribute *var, MethodBody &body,
FmtContext &attrTypeCtx, bool parseAsOptional,
- bool useProperties, StringRef opCppClassName,
+ StringRef opCppClassName,
bool formatAsEnumDirective,
bool formatBitEnumAsUnquoted = false) {
Attribute baseAttr = var->attr.getBaseAttr();
@@ -1310,13 +1300,8 @@ static void genEnumAttrParser(const NamedAttribute *var, MethodBody &body,
errorMessageOS << "]\");";
}
std::string attrAssignment;
- if (useProperties) {
- std::string propAccess = formatv(getPropertiesCode, opCppClassName);
- attrAssignment = formatv(" {0}.{1} = {1}Attr;", propAccess, var->name);
- } else {
- attrAssignment =
- formatv("result.addAttribute(\"{0}\", {0}Attr);", var->name);
- }
+ std::string propAccess = formatv(getPropertiesCode, opCppClassName);
+ attrAssignment = formatv(" {0}.{1} = {1}Attr;", propAccess, var->name);
if (formatAsEnumDirective && enumInfo.isBitEnum() &&
(formatBitEnumAsUnquoted || !enumInfo.printBitEnumQuoted())) {
@@ -1367,14 +1352,13 @@ static void genPropertyParser(PropertyVariable *propVar, MethodBody &body,
// Generate the parser for an attribute.
static void genAttrParser(AttributeVariable *attr, MethodBody &body,
FmtContext &attrTypeCtx, bool parseAsOptional,
- bool useProperties, StringRef opCppClassName) {
+ StringRef opCppClassName) {
const NamedAttribute *var = attr->getVar();
// Check to see if we can parse this as an enum attribute.
if (attr->shouldFormatAsEnum() || canFormatEnumAttr(var))
return genEnumAttrParser(var, body, attrTypeCtx, parseAsOptional,
- useProperties, opCppClassName,
- attr->shouldFormatAsEnum());
+ opCppClassName, attr->shouldFormatAsEnum());
// Check to see if we should parse this as a symbol name attribute.
if (shouldFormatSymbolNameAttr(var)) {
@@ -1402,23 +1386,16 @@ static void genAttrParser(AttributeVariable *attr, MethodBody &body,
body << formatv(attrParserCode, var->name, attrTypeStr);
}
}
- if (useProperties) {
- std::string propAccess = formatv(getPropertiesCode, opCppClassName);
- body << formatv(" if ({0}Attr) {1}.{0} = {0}Attr;\n", var->name,
- propAccess);
- } else {
- body << formatv(
- " if ({0}Attr) result.attributes.append(\"{0}\", {0}Attr);\n",
- var->name);
- }
+ std::string propAccess = formatv(getPropertiesCode, opCppClassName);
+ body << formatv(" if ({0}Attr) {1}.{0} = {0}Attr;\n", var->name, propAccess);
}
// Generates the 'setPropertiesFromParsedAttr' used to set properties from a
// 'prop-dict' dictionary attr.
static void genParsedAttrPropertiesSetter(OperationFormat &fmt, Operator &op,
OpClass &opClass) {
- // Not required unless 'prop-dict' is present or we are not using properties.
- if (!fmt.hasPropDict || !fmt.useProperties)
+ // Not required unless 'prop-dict' is present.
+ if (!fmt.hasPropDict)
return;
SmallVector<MethodParameter> paramList;
@@ -1587,7 +1564,7 @@ static bool isPresenceOnlyUnitProp(const Property &property) {
static void genKeyValuePropDictParser(OperationFormat &fmt, Operator &op,
OpClass &opClass) {
- if (!fmt.hasPropDict || !fmt.useProperties)
+ if (!fmt.hasPropDict)
return;
SmallVector<MethodParameter> paramList;
@@ -1773,8 +1750,7 @@ auto parseResult = ::mlir::detail::parsePropertyWithFallback(
<< " " << attribute.attr.getStorageType() << " "
<< attribute.name << "Attr;\n";
genAttrParser(&attributeVariable, body.indent().indent(), attrTypeCtx,
- /*parseAsOptional=*/false, /*useProperties=*/true,
- fmt.opCppClassName);
+ /*parseAsOptional=*/false, fmt.opCppClassName);
body.unindent() << " } else {\n"
<< " prop." << attribute.name
<< " = parser.getBuilder().getUnitAttr();\n"
@@ -1788,8 +1764,7 @@ auto parseResult = ::mlir::detail::parsePropertyWithFallback(
body << " if (parser.parseLSquare())\n"
" return ::mlir::failure();\n";
genEnumAttrParser(&attribute, body.indent(), attrTypeCtx,
- /*parseAsOptional=*/false, /*useProperties=*/true,
- fmt.opCppClassName,
+ /*parseAsOptional=*/false, fmt.opCppClassName,
/*formatAsEnumDirective=*/true,
/*formatBitEnumAsUnquoted=*/true);
if (useBrackets)
@@ -1797,8 +1772,7 @@ auto parseResult = ::mlir::detail::parsePropertyWithFallback(
" return ::mlir::failure();\n";
} else {
genAttrParser(&attributeVariable, body.indent(), attrTypeCtx,
- /*parseAsOptional=*/false, /*useProperties=*/true,
- fmt.opCppClassName);
+ /*parseAsOptional=*/false, fmt.opCppClassName);
}
}
body.unindent() << " }\n";
@@ -1917,12 +1891,9 @@ void OperationFormat::genElementParser(FormatElement *element, MethodBody &body,
if (isa<PropertyVariable>(anchorVar)) {
body << formatv(" {0}.{1} = true;", propAccess,
anchorVar->getName());
- } else if (useProperties) {
+ } else {
body << formatv(" {0}.{1} = parser.getBuilder().getUnitAttr();",
propAccess, anchorVar->getName());
- } else {
- body << " result.addAttribute(\"" << anchorVar->getName()
- << "\", parser.getBuilder().getUnitAttr());\n";
}
}
}
@@ -1943,7 +1914,7 @@ void OperationFormat::genElementParser(FormatElement *element, MethodBody &body,
FormatElement *firstElement = thenElements.front();
if (auto *attrVar = dyn_cast<AttributeVariable>(firstElement)) {
genAttrParser(attrVar, body, attrTypeCtx, /*parseAsOptional=*/true,
- useProperties, opCppClassName);
+ opCppClassName);
body << " if (" << attrVar->getVar()->name << "Attr) {\n";
} else if (auto *propVar = dyn_cast<PropertyVariable>(firstElement)) {
genPropertyParser(propVar, body, opCppClassName, /*requireParse=*/false);
@@ -1973,7 +1944,7 @@ void OperationFormat::genElementParser(FormatElement *element, MethodBody &body,
}
} else if (auto *custom = dyn_cast<CustomDirective>(firstElement)) {
body << " if (auto optResult = [&]() -> ::mlir::OptionalParseResult {\n";
- genCustomDirectiveParser(custom, body, useProperties, opCppClassName,
+ genCustomDirectiveParser(custom, body, opCppClassName,
/*isOptional=*/true);
body << " return ::mlir::success();\n"
<< " }(); optResult.has_value() && ::mlir::failed(*optResult)) {\n"
@@ -2049,12 +2020,9 @@ void OperationFormat::genElementParser(FormatElement *element, MethodBody &body,
if (isa<PropertyVariable>(unitVarElem)) {
body << formatv(" {0}.{1} = true;", propAccess,
unitVarElem->getName());
- } else if (useProperties) {
+ } else {
body << formatv(" {0}.{1} = parser.getBuilder().getUnitAttr();",
propAccess, unitVarElem->getName());
- } else {
- body << " result.addAttribute(\"" << unitVarElem->getName()
- << "\", UnitAttr::get(parser.getContext()));\n";
}
} else {
for (FormatElement *el : pelement)
@@ -2088,8 +2056,7 @@ void OperationFormat::genElementParser(FormatElement *element, MethodBody &body,
} else if (auto *attr = dyn_cast<AttributeVariable>(element)) {
bool parseAsOptional =
(genCtx == GenContext::Normal && attr->getVar()->attr.isOptional());
- genAttrParser(attr, body, attrTypeCtx, parseAsOptional, useProperties,
- opCppClassName);
+ genAttrParser(attr, body, attrTypeCtx, parseAsOptional, opCppClassName);
} else if (auto *prop = dyn_cast<PropertyVariable>(element)) {
genPropertyParser(prop, body, opCppClassName);
@@ -2131,14 +2098,14 @@ void OperationFormat::genElementParser(FormatElement *element, MethodBody &body,
<< (attrDict->isWithKeyword() ? "WithKeyword" : "")
<< "(result.attributes))\n"
<< " return ::mlir::failure();\n";
- if (useProperties && !useStrictPropertiesInAssemblyFormat) {
+ if (!useStrictPropertiesInAssemblyFormat) {
body << "if (failed(verifyInherentAttrs(result.name, result.attributes, "
"[&]() {\n"
<< " return parser.emitError(loc) << \"'\" << "
"result.name.getStringRef() << \"' op \";\n"
<< " })))\n"
<< " return ::mlir::failure();\n";
- } else if (useProperties) {
+ } else {
for (StringRef name : inherentAttrNames) {
body << "if (result.attributes.get(\"" << name << "\"))\n"
<< " return parser.emitError(loc, \"inherent attribute '" << name
@@ -2149,12 +2116,10 @@ void OperationFormat::genElementParser(FormatElement *element, MethodBody &body,
body.unindent() << "}\n";
body.unindent();
} else if (isa<PropDictDirective>(element)) {
- if (useProperties) {
- body << " if (parseProperties(parser, result))\n"
- << " return ::mlir::failure();\n";
- }
+ body << " if (parseProperties(parser, result))\n"
+ << " return ::mlir::failure();\n";
} else if (auto *customDir = dyn_cast<CustomDirective>(element)) {
- genCustomDirectiveParser(customDir, body, useProperties, opCppClassName);
+ genCustomDirectiveParser(customDir, body, opCppClassName);
} else if (isa<OperandsDirective>(element)) {
body << " [[maybe_unused]] ::llvm::SMLoc allOperandLoc ="
<< " parser.getCurrentLocation();\n"
@@ -2559,7 +2524,7 @@ static void genEnumAttrPrinter(const NamedAttribute *var, const Operator &op,
/// Generate the key-value printer used by the default `prop-dict` printer.
static void genKeyValuePropDictPrinter(OperationFormat &fmt, Operator &op,
OpClass &opClass) {
- if (!fmt.hasPropDict || !fmt.useProperties || op.hasCustomPropertiesPrinter())
+ if (!fmt.hasPropDict || op.hasCustomPropertiesPrinter())
return;
bool hasPrintableField =
@@ -2749,10 +2714,8 @@ static void genPropDictPrinter(OperationFormat &fmt, Operator &op,
// The `printProperties` method is responsible for printing out a leading
// space so that empty `prop-dict`s don't produce stray whitespace.
- if (fmt.useProperties) {
- body << " printProperties((*this)->getContext(), _odsPrinter, "
- "getProperties(), elidedProps);\n";
- }
+ body << " printProperties((*this)->getContext(), _odsPrinter, "
+ "getProperties(), elidedProps);\n";
}
/// Generate the printer for the 'attr-dict' directive.
@@ -3587,8 +3550,7 @@ LogicalResult OpFormatParser::verify(SMLoc loc,
failed(verifyOIListElements(loc, elements)))
return failure();
- if (fmt.useProperties && fmt.useStrictPropertiesInAssemblyFormat &&
- !hasPropDict) {
+ if (fmt.useStrictPropertiesInAssemblyFormat && !hasPropDict) {
auto emitMissingError = [&](StringRef kind,
StringRef name) -> LogicalResult {
return emitError(loc,
@@ -4715,15 +4677,14 @@ LogicalResult OpFormatParser::verifyOptionalGroupElement(SMLoc loc,
// Interface
//===----------------------------------------------------------------------===//
-void mlir::tblgen::generateOpFormat(const Operator &constOp, OpClass &opClass,
- bool hasProperties) {
+void mlir::tblgen::generateOpFormat(const Operator &constOp, OpClass &opClass) {
// TODO: Operator doesn't expose all necessary functionality via
// the const interface.
Operator &op = const_cast<Operator &>(constOp);
if (!op.hasAssemblyFormat()) {
// We still need to generate the parsed attribute properties setter for
// allowing it to be reused in custom assembly implementations.
- OperationFormat format(op, hasProperties);
+ OperationFormat format(op);
format.hasPropDict = true;
genParsedAttrPropertiesSetter(format, op, opClass);
return;
@@ -4733,7 +4694,7 @@ void mlir::tblgen::generateOpFormat(const Operator &constOp, OpClass &opClass,
llvm::SourceMgr mgr;
mgr.AddNewSourceBuffer(
llvm::MemoryBuffer::getMemBuffer(op.getAssemblyFormat()), SMLoc());
- OperationFormat format(op, hasProperties);
+ OperationFormat format(op);
OpFormatParser parser(mgr, format, op);
FailureOr<std::vector<FormatElement *>> elements = parser.parse();
if (failed(elements)) {
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.h b/mlir/tools/mlir-tblgen/OpFormatGen.h
index 5e43f38498664..88dbc99d9f78e 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.h
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.h
@@ -20,8 +20,7 @@ class OpClass;
class Operator;
// Generate the assembly format for the given operator.
-void generateOpFormat(const Operator &constOp, OpClass &opClass,
- bool hasProperties);
+void generateOpFormat(const Operator &constOp, OpClass &opClass);
} // namespace tblgen
} // namespace mlir
More information about the Mlir-commits
mailing list