aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeff Niu <jeff@modular.com>2024-04-04 22:39:07 +0300
committerGitHub <noreply@github.com>2024-04-04 21:39:07 +0200
commitdad065dc6e03725aeb60d703cbaccd175a2f1d53 (patch)
treef459ba6cfb8cc910f4e8779174a5d34e7ab65cdd
parentce46b400d568994c45361c3061554bd2b2e81b8b (diff)
downloadllvm-dad065dc6e03725aeb60d703cbaccd175a2f1d53.zip
llvm-dad065dc6e03725aeb60d703cbaccd175a2f1d53.tar.gz
llvm-dad065dc6e03725aeb60d703cbaccd175a2f1d53.tar.bz2
[mlir][ods] Fix attribute setter gen when properties are on (#87688)
ODS was still generating the old `Operation::setAttr` hooks for ODS methods for setting attributes, when the backing implementation of the attributes was changed to properties. No idea how this wasn't noticed until now.
-rw-r--r--mlir/test/mlir-tblgen/op-properties.td21
-rw-r--r--mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp52
2 files changed, 60 insertions, 13 deletions
diff --git a/mlir/test/mlir-tblgen/op-properties.td b/mlir/test/mlir-tblgen/op-properties.td
new file mode 100644
index 0000000..a484f68
--- /dev/null
+++ b/mlir/test/mlir-tblgen/op-properties.td
@@ -0,0 +1,21 @@
+// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s
+
+include "mlir/IR/AttrTypeBase.td"
+include "mlir/IR/EnumAttr.td"
+include "mlir/IR/OpBase.td"
+
+def Test_Dialect : Dialect {
+ let name = "test";
+ let cppNamespace = "foobar";
+}
+class NS_Op<string mnemonic, list<Trait> traits = []> :
+ Op<Test_Dialect, mnemonic, traits>;
+
+def OpWithAttr : NS_Op<"op_with_attr">{
+ let arguments = (ins AnyAttr:$attr, OptionalAttr<AnyAttr>:$optional);
+}
+
+// CHECK: void OpWithAttr::setAttrAttr(::mlir::Attribute attr)
+// CHECK-NEXT: getProperties().attr = attr
+// CHECK: void OpWithAttr::setOptionalAttr(::mlir::Attribute attr)
+// CHECK-NEXT: getProperties().optional = attr
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 3a69752..843760d 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -1804,23 +1804,36 @@ void OpEmitter::genAttrGetters() {
}
void OpEmitter::genAttrSetters() {
+ bool useProperties = op.getDialect().usePropertiesForAttributes();
+
+ // Generate the code to set an attribute.
+ auto emitSetAttr = [&](Method *method, StringRef getterName,
+ StringRef attrName, StringRef attrVar) {
+ if (useProperties) {
+ method->body() << formatv(" getProperties().{0} = {1};", attrName,
+ attrVar);
+ } else {
+ method->body() << formatv(" (*this)->setAttr({0}AttrName(), {1});",
+ getterName, attrVar);
+ }
+ };
+
// Generate raw named setter type. This is a wrapper class that allows setting
// to the attributes via setters instead of having to use the string interface
// for better compile time verification.
auto emitAttrWithStorageType = [&](StringRef setterName, StringRef getterName,
- Attribute attr) {
+ StringRef attrName, Attribute attr) {
auto *method =
opClass.addMethod("void", setterName + "Attr",
MethodParameter(attr.getStorageType(), "attr"));
if (method)
- method->body() << formatv(" (*this)->setAttr({0}AttrName(), attr);",
- getterName);
+ emitSetAttr(method, getterName, attrName, "attr");
};
// Generate a setter that accepts the underlying C++ type as opposed to the
// attribute type.
auto emitAttrWithReturnType = [&](StringRef setterName, StringRef getterName,
- Attribute attr) {
+ StringRef attrName, Attribute attr) {
Attribute baseAttr = attr.getBaseAttr();
if (!canUseUnwrappedRawValue(baseAttr))
return;
@@ -1849,9 +1862,8 @@ void OpEmitter::genAttrSetters() {
// If the value isn't optional, just set it directly.
if (!isOptional) {
- method->body() << formatv(
- " (*this)->setAttr({0}AttrName(), {1});", getterName,
- constBuildAttrFromParam(attr, fctx, "attrValue"));
+ emitSetAttr(method, getterName, attrName,
+ constBuildAttrFromParam(attr, fctx, "attrValue"));
return;
}
@@ -1862,13 +1874,25 @@ void OpEmitter::genAttrSetters() {
// optional but not in the same way as the others (i.e. it uses bool over
// std::optional<>).
StringRef paramStr = isUnitAttr ? "attrValue" : "*attrValue";
- const char *optionalCodeBody = R"(
+ if (!useProperties) {
+ const char *optionalCodeBody = R"(
if (attrValue)
return (*this)->setAttr({0}AttrName(), {1});
(*this)->removeAttr({0}AttrName());)";
- method->body() << formatv(
- optionalCodeBody, getterName,
- constBuildAttrFromParam(baseAttr, fctx, paramStr));
+ method->body() << formatv(
+ optionalCodeBody, getterName,
+ constBuildAttrFromParam(baseAttr, fctx, paramStr));
+ } else {
+ const char *optionalCodeBody = R"(
+ auto &odsProp = getProperties().{0};
+ if (attrValue)
+ odsProp = {1};
+ else
+ odsProp = nullptr;)";
+ method->body() << formatv(
+ optionalCodeBody, attrName,
+ constBuildAttrFromParam(baseAttr, fctx, paramStr));
+ }
};
for (const NamedAttribute &namedAttr : op.getAttributes()) {
@@ -1876,8 +1900,10 @@ void OpEmitter::genAttrSetters() {
continue;
std::string setterName = op.getSetterName(namedAttr.name);
std::string getterName = op.getGetterName(namedAttr.name);
- emitAttrWithStorageType(setterName, getterName, namedAttr.attr);
- emitAttrWithReturnType(setterName, getterName, namedAttr.attr);
+ emitAttrWithStorageType(setterName, getterName, namedAttr.name,
+ namedAttr.attr);
+ emitAttrWithReturnType(setterName, getterName, namedAttr.name,
+ namedAttr.attr);
}
}