[MLIR][ODS] Check hasProperties when generating populateDefaultAttrs

[zyx-billy]

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.

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Billy Zhu (zyx-billy)

Changes

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 --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");