Skip to content

Commit dad065d

Browse files
author
Jeff Niu
authored
[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.
1 parent ce46b40 commit dad065d

File tree

2 files changed

+60
-13
lines changed

2 files changed

+60
-13
lines changed
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s
2+
3+
include "mlir/IR/AttrTypeBase.td"
4+
include "mlir/IR/EnumAttr.td"
5+
include "mlir/IR/OpBase.td"
6+
7+
def Test_Dialect : Dialect {
8+
let name = "test";
9+
let cppNamespace = "foobar";
10+
}
11+
class NS_Op<string mnemonic, list<Trait> traits = []> :
12+
Op<Test_Dialect, mnemonic, traits>;
13+
14+
def OpWithAttr : NS_Op<"op_with_attr">{
15+
let arguments = (ins AnyAttr:$attr, OptionalAttr<AnyAttr>:$optional);
16+
}
17+
18+
// CHECK: void OpWithAttr::setAttrAttr(::mlir::Attribute attr)
19+
// CHECK-NEXT: getProperties().attr = attr
20+
// CHECK: void OpWithAttr::setOptionalAttr(::mlir::Attribute attr)
21+
// CHECK-NEXT: getProperties().optional = attr

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1804,23 +1804,36 @@ void OpEmitter::genAttrGetters() {
18041804
}
18051805

18061806
void OpEmitter::genAttrSetters() {
1807+
bool useProperties = op.getDialect().usePropertiesForAttributes();
1808+
1809+
// Generate the code to set an attribute.
1810+
auto emitSetAttr = [&](Method *method, StringRef getterName,
1811+
StringRef attrName, StringRef attrVar) {
1812+
if (useProperties) {
1813+
method->body() << formatv(" getProperties().{0} = {1};", attrName,
1814+
attrVar);
1815+
} else {
1816+
method->body() << formatv(" (*this)->setAttr({0}AttrName(), {1});",
1817+
getterName, attrVar);
1818+
}
1819+
};
1820+
18071821
// Generate raw named setter type. This is a wrapper class that allows setting
18081822
// to the attributes via setters instead of having to use the string interface
18091823
// for better compile time verification.
18101824
auto emitAttrWithStorageType = [&](StringRef setterName, StringRef getterName,
1811-
Attribute attr) {
1825+
StringRef attrName, Attribute attr) {
18121826
auto *method =
18131827
opClass.addMethod("void", setterName + "Attr",
18141828
MethodParameter(attr.getStorageType(), "attr"));
18151829
if (method)
1816-
method->body() << formatv(" (*this)->setAttr({0}AttrName(), attr);",
1817-
getterName);
1830+
emitSetAttr(method, getterName, attrName, "attr");
18181831
};
18191832

18201833
// Generate a setter that accepts the underlying C++ type as opposed to the
18211834
// attribute type.
18221835
auto emitAttrWithReturnType = [&](StringRef setterName, StringRef getterName,
1823-
Attribute attr) {
1836+
StringRef attrName, Attribute attr) {
18241837
Attribute baseAttr = attr.getBaseAttr();
18251838
if (!canUseUnwrappedRawValue(baseAttr))
18261839
return;
@@ -1849,9 +1862,8 @@ void OpEmitter::genAttrSetters() {
18491862

18501863
// If the value isn't optional, just set it directly.
18511864
if (!isOptional) {
1852-
method->body() << formatv(
1853-
" (*this)->setAttr({0}AttrName(), {1});", getterName,
1854-
constBuildAttrFromParam(attr, fctx, "attrValue"));
1865+
emitSetAttr(method, getterName, attrName,
1866+
constBuildAttrFromParam(attr, fctx, "attrValue"));
18551867
return;
18561868
}
18571869

@@ -1862,22 +1874,36 @@ void OpEmitter::genAttrSetters() {
18621874
// optional but not in the same way as the others (i.e. it uses bool over
18631875
// std::optional<>).
18641876
StringRef paramStr = isUnitAttr ? "attrValue" : "*attrValue";
1865-
const char *optionalCodeBody = R"(
1877+
if (!useProperties) {
1878+
const char *optionalCodeBody = R"(
18661879
if (attrValue)
18671880
return (*this)->setAttr({0}AttrName(), {1});
18681881
(*this)->removeAttr({0}AttrName());)";
1869-
method->body() << formatv(
1870-
optionalCodeBody, getterName,
1871-
constBuildAttrFromParam(baseAttr, fctx, paramStr));
1882+
method->body() << formatv(
1883+
optionalCodeBody, getterName,
1884+
constBuildAttrFromParam(baseAttr, fctx, paramStr));
1885+
} else {
1886+
const char *optionalCodeBody = R"(
1887+
auto &odsProp = getProperties().{0};
1888+
if (attrValue)
1889+
odsProp = {1};
1890+
else
1891+
odsProp = nullptr;)";
1892+
method->body() << formatv(
1893+
optionalCodeBody, attrName,
1894+
constBuildAttrFromParam(baseAttr, fctx, paramStr));
1895+
}
18721896
};
18731897

18741898
for (const NamedAttribute &namedAttr : op.getAttributes()) {
18751899
if (namedAttr.attr.isDerivedAttr())
18761900
continue;
18771901
std::string setterName = op.getSetterName(namedAttr.name);
18781902
std::string getterName = op.getGetterName(namedAttr.name);
1879-
emitAttrWithStorageType(setterName, getterName, namedAttr.attr);
1880-
emitAttrWithReturnType(setterName, getterName, namedAttr.attr);
1903+
emitAttrWithStorageType(setterName, getterName, namedAttr.name,
1904+
namedAttr.attr);
1905+
emitAttrWithReturnType(setterName, getterName, namedAttr.name,
1906+
namedAttr.attr);
18811907
}
18821908
}
18831909

0 commit comments

Comments
 (0)