Skip to content

Commit

Permalink
[MLIR][ODS] Check hasProperties when generating populateDefaultAttrs (l…
Browse files Browse the repository at this point in the history
…lvm#78525)

Currently ODS generates `populateDefaultAttrs` or
`populateDefaultProperties` based on whether the dialect opted into
usePropertiesForAttributes. But since individual ops might get opted
into using properties (as long as it has one property), it should
actually just check whether the op itself uses properties. Otherwise
`populateDefaultAttrs` will overwrite existing attrs inside properties
when creating an op. Understandably this becomes moot once everything
switches over to using properties, but this fixes it for now.

This PR makes ODS generate `populateDefaultProperties` as long as the op
itself uses properties.
  • Loading branch information
zyx-billy authored Jan 19, 2024
1 parent dbc0955 commit 595d780
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
13 changes: 13 additions & 0 deletions mlir/test/mlir-tblgen/op-attribute.td
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ def AOp : NS_Op<"a_op", []> {
// DEF: ::llvm::ArrayRef<::mlir::NamedAttribute> attributes
// DEF: odsState.addAttributes(attributes);

// DEF: void AOp::populateDefaultAttrs

// Test the above but with prefix.

def Test2_Dialect : Dialect {
Expand Down Expand Up @@ -287,6 +289,17 @@ def AgetOp : Op<Test2_Dialect, "a_get_op", []> {
// DEF: ::llvm::ArrayRef<::mlir::NamedAttribute> attributes
// DEF: odsState.addAttributes(attributes);

// Test the above but using properties.
def ApropOp : NS_Op<"a_prop_op", []> {
let arguments = (ins
Property<"unsigned">:$aAttr,
DefaultValuedAttr<SomeAttr, "4.2">:$bAttr
);
}

// DEF-LABEL: ApropOp definitions
// DEF: void ApropOp::populateDefaultProperties

def SomeTypeAttr : TypeAttrBase<"SomeType", "some type attribute">;

def BOp : NS_Op<"b_op", []> {
Expand Down
2 changes: 1 addition & 1 deletion mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2506,7 +2506,7 @@ void OpEmitter::genPopulateDefaultAttributes() {
}))
return;

if (op.getDialect().usePropertiesForAttributes()) {
if (emitHelper.hasProperties()) {
SmallVector<MethodParameter> paramList;
paramList.emplace_back("::mlir::OperationName", "opName");
paramList.emplace_back("Properties &", "properties");
Expand Down

0 comments on commit 595d780

Please sign in to comment.