[Mlir-commits] [mlir] [MLIR][ODS] Check hasProperties when generating populateDefaultAttrs (PR #78525)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-core

Author: Billy Zhu (zyx-billy)

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/78525.diff


2 Files Affected:

- (modified) mlir/test/mlir-tblgen/op-attribute.td (+13) 
- (modified) mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp (+1-1) 


``````````diff
diff --git a/mlir/test/mlir-tblgen/op-attribute.td b/mlir/test/mlir-tblgen/op-attribute.td
index 07636f53c1e15c..b5b8619e7c9bea 100644
--- a/mlir/test/mlir-tblgen/op-attribute.td
+++ b/mlir/test/mlir-tblgen/op-attribute.td
@@ -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 {
@@ -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", []> {
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index cd37c8dcd3d5e0..71326049af0579 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -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");

``````````

</details>


https://github.com/llvm/llvm-project/pull/78525