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

[llvmlistbot at llvm.org]

Author: Billy Zhu
Date: 2024-01-18T21:57:20-08:00
New Revision: 595d780b492d3de3f5653851e7d64c122472d71a

URL: https://github.com/llvm/llvm-project/commit/595d780b492d3de3f5653851e7d64c122472d71a
DIFF: https://github.com/llvm/llvm-project/commit/595d780b492d3de3f5653851e7d64c122472d71a.diff

LOG: [MLIR][ODS] Check hasProperties when generating populateDefaultAttrs (#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.

Added: 
    

Modified: 
    mlir/test/mlir-tblgen/op-attribute.td
    mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/test/mlir-tblgen/op-attribute.td b/mlir/test/mlir-tblgen/op-attribute.td
index 07636f53c1e15ce..b5b8619e7c9beaf 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 cd37c8dcd3d5e0e..71326049af05796 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");