[Mlir-commits] [mlir] [mlir][ODS] Populate properties in legacy aggregate builders (PR #219194)

Mehdi Amini llvmlistbot at llvm.org
Thu Aug 27 05:07:22 PDT 2026


https://github.com/joker-eph created https://github.com/llvm/llvm-project/pull/219194

Partition mixed aggregate attributes before operation creation and deprecate the generated compatibility overloads for operations with non-empty properties.

Assisted-by: Codex

>From 12b04fb8afc2f913ec943a1e624f8f5f89cadfe1 Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Thu, 20 Aug 2026 06:54:19 -0700
Subject: [PATCH] [mlir][ODS] Populate properties in legacy aggregate builders

Partition mixed aggregate attributes before operation creation and deprecate
the generated compatibility overloads for operations with non-empty
properties.

Assisted-by: Codex
---
 mlir/docs/DefiningDialects/Operations.md    |  20 +-
 mlir/test/lib/Dialect/Test/TestOps.td       |  51 ++++
 mlir/test/mlir-tblgen/op-attribute.td       |   4 +-
 mlir/test/mlir-tblgen/op-decl-and-defs.td   | 108 +++++++--
 mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp | 254 +++++++++++++-------
 mlir/unittests/TableGen/OpBuildGen.cpp      | 115 ++++++++-
 6 files changed, 434 insertions(+), 118 deletions(-)

diff --git a/mlir/docs/DefiningDialects/Operations.md b/mlir/docs/DefiningDialects/Operations.md
index ee427d45c3780..dc64edd75cd59 100644
--- a/mlir/docs/DefiningDialects/Operations.md
+++ b/mlir/docs/DefiningDialects/Operations.md
@@ -483,7 +483,7 @@ The following builders are generated:
 static void build(OpBuilder &odsBuilder, OperationState &odsState,
                   TypeRange resultTypes,
                   ValueRange operands,
-                  Properties properties,
+                  const Properties &properties,
                   ArrayRef<NamedAttribute> discardableAttributes = {});
 
 // All result-types/operands/attributes have one aggregate parameter.
@@ -523,7 +523,7 @@ static void build(OpBuilder &odsBuilder, OperationState &odsState,
 // Generated if return type can be inferred.
 static void build(OpBuilder &odsBuilder, OperationState &odsState,
                   ValueRange operands,
-                  Properties properties,
+                  const Properties &properties,
                   ArrayRef<NamedAttribute> discardableAttributes);
 
 // All operands/attributes have aggregate parameters.
@@ -539,6 +539,22 @@ The first two forms provide basic uniformity so that we can create ops using
 the same form regardless of the exact op. This is particularly useful for
 implementing declarative pattern rewrites.
 
+For operations with non-empty properties, the aggregate builder that takes a
+mixed `attributes` array is deprecated. Use the overload that takes a typed
+`Properties` structure and a separate `discardableAttributes` array instead.
+The deprecated overload remains available for compatibility: it partitions the
+mixed array using the operation's statically known inherent-attribute and
+property names, converts that subset into `Properties`, and places only the
+remaining discardable attributes in `OperationState::attributes`. Defaults and
+result-type inference therefore observe the populated properties before the
+operation is created. Operations with empty properties retain the ordinary
+aggregate attribute builder without a deprecation.
+
+This applies to all aggregate builder variants, including builders with
+explicit or inferred result types and builders that derive result types from
+operands or the first attribute. The overload taking `Properties` and
+`discardableAttributes` is not deprecated.
+
 The third and fourth forms are good for use in manually written code, given that
 they provide better guarantee via signatures.
 
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index dc0da7b54c90b..d6c1caefd1a19 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -3019,6 +3019,57 @@ def TableGenBuildOp7 : TEST_Op<"tblgen_build_7", []> {
   let results = (outs);
 }
 
+// Properties of every supported kind on an inferred-result aggregate builder.
+// The inference hook checks that the legacy mixed-attribute overload populated
+// defaults and converted properties before invoking it.
+def TableGenBuildOp8 : TEST_Op<"tblgen_build_8", [
+    InferTypeOpInterface, AttrSizedOperandSegments, AttrSizedResultSegments
+  ]> {
+  let arguments = (ins
+    Variadic<AnyType>:$a,
+    Variadic<AnyType>:$b,
+    BoolAttr:$attr0,
+    DefaultValuedAttr<I32Attr, "7">:$defaultAttr,
+    I64Prop:$nativeProp
+  );
+  let results = (outs Variadic<AnyType>:$resultA,
+                      Variadic<AnyType>:$resultB);
+
+  let extraClassDeclaration = [{
+    static ::llvm::LogicalResult inferReturnTypes(
+        ::mlir::MLIRContext *, ::std::optional<::mlir::Location>,
+        ::mlir::ValueRange operands, ::mlir::DictionaryAttr,
+        ::mlir::PropertyRef properties, ::mlir::RegionRange,
+        ::llvm::SmallVectorImpl<::mlir::Type> &inferredReturnTypes) {
+      if (!properties)
+        return ::mlir::failure();
+      const auto &prop = *properties.as<const Properties *>();
+      if (!prop.attr0 || !prop.attr0.getValue() || !prop.defaultAttr ||
+          prop.defaultAttr.getInt() != 7 || prop.nativeProp != 42 ||
+          prop.operandSegmentSizes != std::array<int32_t, 2>{1, 1} ||
+          prop.resultSegmentSizes != std::array<int32_t, 2>{1, 0})
+        return ::mlir::failure();
+      inferredReturnTypes.assign({operands.front().getType()});
+      return ::mlir::success();
+    }
+  }];
+}
+
+// Exercise the aggregate builder that derives results from operand types.
+def TableGenBuildOp9 : TEST_Op<"tblgen_build_9",
+    [SameOperandsAndResultType]> {
+  let arguments = (ins AnyType:$input, BoolAttr:$attr0);
+  let results = (outs AnyType:$result);
+}
+
+// Exercise the aggregate builder that derives results from its first
+// attribute after that attribute has been converted to properties.
+def TableGenBuildOp10 : TEST_Op<"tblgen_build_10",
+    [FirstAttrDerivedResultType]> {
+  let arguments = (ins TypeAttr:$type, AnyType:$input);
+  let results = (outs AnyType:$result);
+}
+
 //===----------------------------------------------------------------------===//
 // Test BufferPlacement
 //===----------------------------------------------------------------------===//
diff --git a/mlir/test/mlir-tblgen/op-attribute.td b/mlir/test/mlir-tblgen/op-attribute.td
index cfdaaebb9e91e..ebba86d32d82f 100644
--- a/mlir/test/mlir-tblgen/op-attribute.td
+++ b/mlir/test/mlir-tblgen/op-attribute.td
@@ -163,7 +163,7 @@ def AOp : NS_Op<"a_op", []> {
 
 // DEF:      void AOp::build(
 // DEF:        ::llvm::ArrayRef<::mlir::NamedAttribute> attributes
-// DEF:      odsState.addAttributes(attributes);
+// DEF:      buildPropertiesAndDiscardableAttributes(odsState, attributes);
 
 // DEF:      void AOp::build(
 // DEF-SAME:   const Properties &properties,
@@ -283,7 +283,7 @@ def AgetOp : Op<Test2_Dialect, "a_get_op", []> {
 
 // DEF:      void AgetOp::build(
 // DEF:        ::llvm::ArrayRef<::mlir::NamedAttribute> attributes
-// DEF:      odsState.addAttributes(attributes);
+// DEF:      buildPropertiesAndDiscardableAttributes(odsState, attributes);
 
 // DEF:      void AgetOp::build(
 // DEF-SAME:   const Properties &properties
diff --git a/mlir/test/mlir-tblgen/op-decl-and-defs.td b/mlir/test/mlir-tblgen/op-decl-and-defs.td
index cd1d471773abc..32c4bb88d188b 100644
--- a/mlir/test/mlir-tblgen/op-decl-and-defs.td
+++ b/mlir/test/mlir-tblgen/op-decl-and-defs.td
@@ -135,23 +135,40 @@ def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> {
 // CHECK:   static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::Value a, ::mlir::ValueRange b, uint32_t attr1, /*optional*/::mlir::FloatAttr some_attr2, unsigned someRegionsCount);
 // CHECK:   static AOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange resultTypes, ::mlir::Value a, ::mlir::ValueRange b, uint32_t attr1, /*optional*/::mlir::FloatAttr some_attr2, unsigned someRegionsCount);
 // CHECK:   static AOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange resultTypes, ::mlir::Value a, ::mlir::ValueRange b, uint32_t attr1, /*optional*/::mlir::FloatAttr some_attr2, unsigned someRegionsCount);
-// CHECK:   static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes, unsigned numRegions)
-// CHECK:   static AOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes, unsigned numRegions)
-// CHECK:   static AOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes, unsigned numRegions)
-// CHECK:   static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes, unsigned numRegions)
-// CHECK:   static AOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes, unsigned numRegions)
-// CHECK:   static AOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes, unsigned numRegions)
+// CHECK{LITERAL}:   [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT:   static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes, unsigned numRegions)
+// CHECK{LITERAL}:   [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT:   static AOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes, unsigned numRegions)
+// CHECK{LITERAL}:   [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT:   static AOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes, unsigned numRegions)
+// CHECK-NEXT:   static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes, unsigned numRegions)
+// CHECK-NEXT:   static AOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes, unsigned numRegions)
+// CHECK-NEXT:   static AOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes, unsigned numRegions)
 // CHECK:   static ::mlir::ParseResult parse(::mlir::OpAsmParser &parser, ::mlir::OperationState &result);
 // CHECK:   void print(::mlir::OpAsmPrinter &p);
 // CHECK:   ::llvm::LogicalResult verifyInvariants();
 // CHECK:   static void getCanonicalizationPatterns(::mlir::RewritePatternSet &results, ::mlir::MLIRContext *context);
 // CHECK:   ::llvm::LogicalResult fold(FoldAdaptor adaptor, ::llvm::SmallVectorImpl<::mlir::OpFoldResult> &results);
 // CHECK:   static ::llvm::LogicalResult setPropertiesFromParsedAttr(Properties &prop, ::mlir::Attribute attr, ::llvm::function_ref<::mlir::InFlightDiagnostic()> emitError);
+// CHECK: private:
+// CHECK:   static void buildPropertiesAndDiscardableAttributes(::mlir::OperationState &odsState, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes);
+// CHECK-NEXT: public:
 // CHECK:   // Display a graph for debugging purposes.
 // CHECK:   void displayGraph();
 // CHECK: };
 
 // DEFS-LABEL: NS::AOp definitions
+// DEFS: void AOp::build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes, unsigned numRegions) {
+// DEFS:   buildPropertiesAndDiscardableAttributes(odsState, attributes);
+// DEFS: void AOp::build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes, unsigned numRegions) {
+// DEFS:   odsState.useProperties(const_cast<Properties&>(properties));
+// DEFS-LABEL: void AOp::buildPropertiesAndDiscardableAttributes
+// DEFS:   Properties &properties = odsState.getOrAddProperties<Properties>();
+// DEFS-NEXT:   populateDefaultProperties(odsState.name, properties);
+// DEFS:     if (name == "attr1" || name == "some_attr2")
+// DEFS:       inherentAttributes.push_back(attr);
+// DEFS:       odsState.addAttribute(attr.getName(), attr.getValue());
+// DEFS:   if (::mlir::failed(setPropertiesFromAttr(
 
 // Check that `getAttrDictionary()` is used when not using properties.
 
@@ -245,6 +262,32 @@ def NS_FOp : NS_Op<"op_with_all_types_constraint",
 // DEFS:   return create(builder, builder.getLoc(), std::forward<decltype(a)>(a));
 // DEFS: }
 
+def NS_FirstAttrDerivedOp : NS_Op<"first_attr_derived",
+    [FirstAttrDerivedResultType]> {
+  let arguments = (ins TypeAttr:$type, AnyType:$input);
+  let results = (outs AnyType:$result);
+}
+
+// CHECK-LABEL: class FirstAttrDerivedOp :
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static FirstAttrDerivedOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static FirstAttrDerivedOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK-NEXT: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK-NEXT: static FirstAttrDerivedOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK-NEXT: static FirstAttrDerivedOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static FirstAttrDerivedOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static FirstAttrDerivedOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK-NEXT: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK-NEXT: static FirstAttrDerivedOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK-NEXT: static FirstAttrDerivedOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+
 def NS_GOp : NS_Op<"op_with_fixed_return_type", []> {
   let arguments = (ins AnyType:$a);
   let results = (outs I32:$b);
@@ -286,6 +329,7 @@ def NS_HCollectiveParamsSuppress0Op : NS_Op<"op_collective_suppress0", []> {
 // CHECK-NOT: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::TypeRange b, ::mlir::ValueRange a);
 // CHECK-NOT: static HCollectiveParamsSuppress0Op create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange b, ::mlir::ValueRange a);
 // CHECK-NOT: static HCollectiveParamsSuppress0Op create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange b, ::mlir::ValueRange a);
+// CHECK-NOT: use the overload taking Properties and discardableAttributes instead
 // CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
 // CHECK: static HCollectiveParamsSuppress0Op create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
 // CHECK: static HCollectiveParamsSuppress0Op create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
@@ -445,21 +489,24 @@ def NS_LOp : NS_Op<"op_with_same_operands_and_result_types_unwrapped_attr", [Sam
 // CHECK: static LOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange resultTypes, ::mlir::Value a, ::mlir::Value b, uint32_t attr1);
 // CHECK: static LOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange resultTypes, ::mlir::Value a, ::mlir::Value b, uint32_t attr1);
 
-// CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
-// CHECK: static LOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
-// CHECK: static LOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
-
-// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
-// CHECK: static LOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
-// CHECK: static LOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
-
-// CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
-// CHECK: static LOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
-// CHECK: static LOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
-
-// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
-// CHECK: static LOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
-// CHECK: static LOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static LOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static LOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static LOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static LOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK-NEXT: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK-NEXT: static LOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK-NEXT: static LOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK-NEXT: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK-NEXT: static LOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK-NEXT: static LOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
 
 def NS_MOp : NS_Op<"op_with_single_result_and_fold_adaptor_fold", []> {
   let results = (outs AnyType:$res);
@@ -551,3 +598,22 @@ def _TypeInferredPropOp : NS_Op<"type_inferred_prop_op_with_properties", [
   let results = (outs AnyType:$result);
   let hasCustomAssemblyFormat = 1;
 }
+
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static _TypeInferredPropOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static _TypeInferredPropOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static _TypeInferredPropOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK{LITERAL}: [[deprecated("use the overload taking Properties and discardableAttributes instead")]]
+// CHECK-NEXT: static _TypeInferredPropOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
+// CHECK-NEXT: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK-NEXT: static _TypeInferredPropOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK-NEXT: static _TypeInferredPropOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK-NEXT: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK-NEXT: static _TypeInferredPropOp create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
+// CHECK-NEXT: static _TypeInferredPropOp create(::mlir::ImplicitLocOpBuilder &builder, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 6d7ec67e46dde..1cb51f52c3969 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -62,6 +62,8 @@ static const char *const odsBuilder = "odsBuilder";
 static const char *const builderOpState = "odsState";
 static const char *const builderOpStateProperties =
     "odsState.getOrAddProperties<Properties>()";
+static constexpr StringLiteral legacyBuilderDeprecation =
+    "use the overload taking Properties and discardableAttributes instead";
 static const char *const propertyStorage = "propStorage";
 static const char *const propertyValue = "propValue";
 static const char *const propertyAttr = "propAttr";
@@ -71,6 +73,10 @@ static const char *const propertyDiag = "emitError";
 /// result segment sizes.
 static const char *const operandSegmentAttrName = "operandSegmentSizes";
 static const char *const resultSegmentAttrName = "resultSegmentSizes";
+static constexpr StringLiteral legacyOperandSegmentAttrName =
+    "operand_segment_sizes";
+static constexpr StringLiteral legacyResultSegmentAttrName =
+    "result_segment_sizes";
 
 /// Code for an Op to lookup an attribute. Uses cached identifiers and subrange
 /// lookup.
@@ -555,15 +561,12 @@ void OpOrAdaptorHelper::computeAttrMetadata() {
   // Store the position of the legacy operand_segment_sizes /
   // result_segment_sizes so we can emit a backward compatible property readers
   // and writers.
-  StringRef legacyOperandSegmentSizeName =
-      StringLiteral("operand_segment_sizes");
-  StringRef legacyResultSegmentSizeName = StringLiteral("result_segment_sizes");
   operandSegmentSizesLegacyIndex = 0;
   resultSegmentSizesLegacyIndex = 0;
   for (auto item : sortedAttrMetadata) {
-    if (item.attrName < legacyOperandSegmentSizeName)
+    if (item.attrName < legacyOperandSegmentAttrName)
       ++operandSegmentSizesLegacyIndex;
-    if (item.attrName < legacyResultSegmentSizeName)
+    if (item.attrName < legacyResultSegmentAttrName)
       ++resultSegmentSizesLegacyIndex;
   }
 
@@ -603,6 +606,10 @@ class OpEmitter {
   OpEmitter(const Operator &op,
             const StaticVerifierFunctionEmitter &staticVerifierEmitter);
 
+  // Returns the inherent attributes and properties represented by the
+  // operation's Properties struct.
+  SmallVector<ConstArgument> getAttrOrProperties();
+
   void emitDecl(raw_ostream &os);
   void emitDef(raw_ostream &os);
 
@@ -661,7 +668,8 @@ class OpEmitter {
   // Generates the build() method that takes each operand/attribute
   // as a stand-alone parameter.
   void genSeparateArgParamBuilder();
-  void genInlineCreateBody(const SmallVector<MethodParameter> &paramList);
+  void genInlineCreateBody(const SmallVector<MethodParameter> &paramList,
+                           bool deprecated = false);
 
   // Generates the build() method that takes each operand/attribute as a
   // stand-alone parameter. The generated build() method uses first operand's
@@ -676,6 +684,15 @@ class OpEmitter {
                 // dictionary
   };
 
+  // Generates the internal helper used by legacy aggregate builders to split
+  // inherent properties from discardable attributes.
+  void genLegacyPropertiesBuilderHelper();
+
+  // Adds aggregate properties and attributes to the operation state.
+  void genCodeForAddingPropertiesAndAttributes(MethodBody &body,
+                                               CollectiveBuilderKind kind,
+                                               StringRef attributesName);
+
   // Generates the build() method that takes all operands/attributes
   // collectively as one parameter. The generated build() method uses first
   // operand's type as all results' types.
@@ -1353,10 +1370,7 @@ static void emitAttrGetterWithReturnType(FmtContext &fctx,
        << ";\n";
 }
 
-void OpEmitter::genPropertiesSupport() {
-  if (!emitHelper.hasProperties())
-    return;
-
+SmallVector<OpEmitter::ConstArgument> OpEmitter::getAttrOrProperties() {
   SmallVector<ConstArgument> attrOrProperties;
   for (const std::pair<StringRef, AttributeMetadata> &it :
        emitHelper.getAttrMetadata()) {
@@ -1369,6 +1383,14 @@ void OpEmitter::genPropertiesSupport() {
     attrOrProperties.push_back(&emitHelper.getOperandSegmentsSize().value());
   if (emitHelper.getResultSegmentsSize())
     attrOrProperties.push_back(&emitHelper.getResultSegmentsSize().value());
+  return attrOrProperties;
+}
+
+void OpEmitter::genPropertiesSupport() {
+  if (!emitHelper.hasProperties())
+    return;
+
+  SmallVector<ConstArgument> attrOrProperties = getAttrOrProperties();
   auto &setPropMethod =
       opClass
           .addStaticMethod(
@@ -1472,11 +1494,13 @@ void OpEmitter::genPropertiesSupport() {
       os << "   auto attr = dict.get(\"" << name << "\");";
       if (name == operandSegmentAttrName) {
         // Backward compat for now, TODO: Remove at some point.
-        os << "   if (!attr) attr = dict.get(\"operand_segment_sizes\");";
+        os << "   if (!attr) attr = dict.get(\"" << legacyOperandSegmentAttrName
+           << "\");";
       }
       if (name == resultSegmentAttrName) {
         // Backward compat for now, TODO: Remove at some point.
-        os << "   if (!attr) attr = dict.get(\"result_segment_sizes\");";
+        os << "   if (!attr) attr = dict.get(\"" << legacyResultSegmentAttrName
+           << "\");";
       }
 
       fctx.withBuilder(odsBuilder);
@@ -1506,11 +1530,13 @@ void OpEmitter::genPropertiesSupport() {
       os << "   auto attr = dict.get(\"" << name << "\");";
       if (name == operandSegmentAttrName) {
         // Backward compat for now
-        os << "   if (!attr) attr = dict.get(\"operand_segment_sizes\");";
+        os << "   if (!attr) attr = dict.get(\"" << legacyOperandSegmentAttrName
+           << "\");";
       }
       if (name == resultSegmentAttrName) {
         // Backward compat for now
-        os << "   if (!attr) attr = dict.get(\"result_segment_sizes\");";
+        os << "   if (!attr) attr = dict.get(\"" << legacyResultSegmentAttrName
+           << "\");";
       }
 
       setPropMethod << formatv(R"decl(
@@ -1663,14 +1689,12 @@ void OpEmitter::genPropertiesSupport() {
     fctx.addSubst("_storage", Twine("prop.") + name);
     if (name == operandSegmentAttrName) {
       getInherentAttrMethod
-          << formatv("    if (name == \"operand_segment_sizes\" || name == "
-                     "\"{0}\") return ",
-                     operandSegmentAttrName);
+          << formatv("    if (name == \"{0}\" || name == \"{1}\") return ",
+                     legacyOperandSegmentAttrName, operandSegmentAttrName);
     } else {
       getInherentAttrMethod
-          << formatv("    if (name == \"result_segment_sizes\" || name == "
-                     "\"{0}\") return ",
-                     resultSegmentAttrName);
+          << formatv("    if (name == \"{0}\" || name == \"{1}\") return ",
+                     legacyResultSegmentAttrName, resultSegmentAttrName);
     }
     getInherentAttrMethod << "[&]() -> ::mlir::Attribute { "
                           << tgfmt(prop.getConvertToAttributeCall(), &fctx)
@@ -1678,14 +1702,12 @@ void OpEmitter::genPropertiesSupport() {
 
     if (name == operandSegmentAttrName) {
       setInherentAttrMethod
-          << formatv("        if (name == \"operand_segment_sizes\" || name == "
-                     "\"{0}\") {{",
-                     operandSegmentAttrName);
+          << formatv("        if (name == \"{0}\" || name == \"{1}\") {{",
+                     legacyOperandSegmentAttrName, operandSegmentAttrName);
     } else {
       setInherentAttrMethod
-          << formatv("        if (name == \"result_segment_sizes\" || name == "
-                     "\"{0}\") {{",
-                     resultSegmentAttrName);
+          << formatv("        if (name == \"{0}\" || name == \"{1}\") {{",
+                     legacyResultSegmentAttrName, resultSegmentAttrName);
     }
     setInherentAttrMethod << formatv(R"decl(
        auto arrAttr = ::llvm::dyn_cast_or_null<::mlir::DenseI32ArrayAttr>(value);
@@ -2529,7 +2551,7 @@ static bool canInferType(const Operator &op) {
 }
 
 void OpEmitter::genInlineCreateBody(
-    const SmallVector<MethodParameter> &paramList) {
+    const SmallVector<MethodParameter> &paramList, bool deprecated) {
   SmallVector<MethodParameter> createParamListOpBuilder;
   SmallVector<MethodParameter> createParamListImplicitLocOpBuilder;
   SmallVector<llvm::StringRef, 4> nonBuilderStateArgsList;
@@ -2560,6 +2582,12 @@ void OpEmitter::genInlineCreateBody(
                                            createParamListOpBuilder);
   auto *cImplicitLoc = opClass.addStaticMethod(
       opClass.getClassName(), "create", createParamListImplicitLocOpBuilder);
+  if (deprecated) {
+    if (cWithLoc)
+      cWithLoc->setDeprecated(legacyBuilderDeprecation);
+    if (cImplicitLoc)
+      cImplicitLoc->setDeprecated(legacyBuilderDeprecation);
+  }
   std::string nonBuilderStateArgs = "";
   if (!nonBuilderStateArgsList.empty()) {
     llvm::raw_string_ostream nonBuilderStateArgsOS(nonBuilderStateArgs);
@@ -2695,6 +2723,82 @@ void OpEmitter::genSeparateArgParamBuilder() {
   }
 }
 
+void OpEmitter::genLegacyPropertiesBuilderHelper() {
+  if (!emitHelper.hasNonEmptyPropertiesStruct())
+    return;
+
+  SmallVector<StringRef> inherentNames;
+  for (const ConstArgument &attrOrProperty : getAttrOrProperties()) {
+    if (const auto *namedAttr =
+            dyn_cast_if_present<const AttributeMetadata *>(attrOrProperty))
+      inherentNames.push_back(namedAttr->attrName);
+    else
+      inherentNames.push_back(
+          cast<const NamedProperty *>(attrOrProperty)->name);
+  }
+  if (emitHelper.getOperandSegmentsSize()) {
+    inherentNames.push_back(legacyOperandSegmentAttrName);
+  }
+  if (emitHelper.getResultSegmentsSize()) {
+    inherentNames.push_back(legacyResultSegmentAttrName);
+  }
+  llvm::sort(inherentNames);
+  inherentNames.erase(llvm::unique(inherentNames), inherentNames.end());
+
+  auto *method = opClass.addStaticMethod<Method::Private>(
+      "void", "buildPropertiesAndDiscardableAttributes",
+      MethodParameter("::mlir::OperationState &", builderOpState),
+      MethodParameter("::llvm::ArrayRef<::mlir::NamedAttribute>",
+                      "attributes"));
+  ERROR_IF_PRUNED(method, "buildPropertiesAndDiscardableAttributes", op);
+  MethodBody &body = method->body();
+  body << "  Properties &properties = " << builderOpStateProperties << ";\n"
+       << "  populateDefaultProperties(" << builderOpState
+       << ".name, properties);\n"
+       << "  ::llvm::SmallVector<::mlir::NamedAttribute> "
+          "inherentAttributes;\n"
+       << "  for (const ::mlir::NamedAttribute &attr : attributes) {\n"
+       << "    ::llvm::StringRef name = attr.getName().getValue();\n"
+       << "    if (";
+  llvm::interleave(
+      inherentNames,
+      [&](StringRef name) { body << "name == \"" << name << "\""; },
+      [&] { body << " || "; });
+  body << ")\n"
+       << "      inherentAttributes.push_back(attr);\n"
+       << "    else\n"
+       << "      " << builderOpState << ".addAttribute(attr.getName(), "
+       << "attr.getValue());\n"
+       << "  }\n"
+       << "  if (inherentAttributes.empty())\n"
+       << "    return;\n"
+       << "  if (::mlir::failed(setPropertiesFromAttr(\n"
+       << "          properties,\n"
+       << "          ::mlir::DictionaryAttr::get(" << builderOpState
+       << ".getContext(), inherentAttributes),\n"
+       << "          [&]() { return ::mlir::emitError(" << builderOpState
+       << ".location); })))\n"
+       << "    ::llvm::report_fatal_error(\"Property conversion failed.\");\n";
+}
+
+void OpEmitter::genCodeForAddingPropertiesAndAttributes(
+    MethodBody &body, CollectiveBuilderKind kind, StringRef attributesName) {
+  if (kind == CollectiveBuilderKind::PropStruct) {
+    body << "  " << builderOpState
+         << ".useProperties(const_cast<Properties&>(properties));\n"
+         << "  " << builderOpState << ".addAttributes(" << attributesName
+         << ");\n";
+    return;
+  }
+  if (emitHelper.hasNonEmptyPropertiesStruct()) {
+    body << "  buildPropertiesAndDiscardableAttributes(" << builderOpState
+         << ", " << attributesName << ");\n";
+    return;
+  }
+  body << "  " << builderOpState << ".addAttributes(" << attributesName
+       << ");\n";
+}
+
 void OpEmitter::genUseOperandAsResultTypeCollectiveParamBuilder(
     CollectiveBuilderKind kind) {
   int numResults = op.getNumResults();
@@ -2720,18 +2824,17 @@ void OpEmitter::genUseOperandAsResultTypeCollectiveParamBuilder(
   // If the builder is redundant, skip generating the method
   if (!m)
     return;
-  genInlineCreateBody(paramList);
+  bool deprecated = kind == CollectiveBuilderKind::AttrDict &&
+                    emitHelper.hasNonEmptyPropertiesStruct();
+  if (deprecated)
+    m->setDeprecated(legacyBuilderDeprecation);
+  genInlineCreateBody(paramList, deprecated);
   auto &body = m->body();
 
   // Operands
   body << "  " << builderOpState << ".addOperands(operands);\n";
 
-  if (kind == CollectiveBuilderKind::PropStruct)
-    body << "  " << builderOpState
-         << ".useProperties(const_cast<Properties&>(properties));\n";
-  // Attributes
-  body << "  " << builderOpState << ".addAttributes(" << attributesName
-       << ");\n";
+  genCodeForAddingPropertiesAndAttributes(body, kind, attributesName);
 
   // Create the correct number of regions
   if (int numRegions = op.getNumRegions()) {
@@ -2835,7 +2938,11 @@ void OpEmitter::genInferredTypeCollectiveParamBuilder(
   // If the builder is redundant, skip generating the method
   if (!m)
     return;
-  genInlineCreateBody(paramList);
+  bool deprecated = kind == CollectiveBuilderKind::AttrDict &&
+                    emitHelper.hasNonEmptyPropertiesStruct();
+  if (deprecated)
+    m->setDeprecated(legacyBuilderDeprecation);
+  genInlineCreateBody(paramList, deprecated);
   auto &body = m->body();
 
   int numResults = op.getNumResults();
@@ -2853,11 +2960,7 @@ void OpEmitter::genInferredTypeCollectiveParamBuilder(
          << numNonVariadicOperands
          << "u && \"mismatched number of parameters\");\n";
   body << "  " << builderOpState << ".addOperands(operands);\n";
-  if (kind == CollectiveBuilderKind::PropStruct)
-    body << "  " << builderOpState
-         << ".useProperties(const_cast<Properties &>(properties));\n";
-  body << "  " << builderOpState << ".addAttributes(" << attributesName
-       << ");\n";
+  genCodeForAddingPropertiesAndAttributes(body, kind, attributesName);
 
   // Create the correct number of regions
   if (int numRegions = op.getNumRegions()) {
@@ -2868,22 +2971,6 @@ void OpEmitter::genInferredTypeCollectiveParamBuilder(
   }
 
   // Result types
-  if (emitHelper.hasNonEmptyPropertiesStruct() &&
-      kind == CollectiveBuilderKind::AttrDict) {
-    // Initialize the properties from Attributes before invoking the infer
-    // function.
-    body << formatv(R"(
-  if (!attributes.empty()) {
-    (void){1}.getOrAddProperties<{0}::Properties>();
-    ::mlir::PropertyRef properties = {1}.getRawProperties();
-    std::optional<::mlir::RegisteredOperationName> info =
-      {1}.name.getRegisteredInfo();
-    if (failed(info->setOpPropertiesFromAttribute({1}.name, properties,
-        {1}.attributes.getDictionary({1}.getContext()), nullptr)))
-      ::llvm::report_fatal_error("Property conversion failed.");
-  })",
-                    opClass.getClassName(), builderOpState);
-  }
   body << formatv(R"(
   ::llvm::SmallVector<::mlir::Type, 2> inferredReturnTypes;
   if (::mlir::succeeded({0}::inferReturnTypes(odsBuilder.getContext(),
@@ -2959,7 +3046,11 @@ void OpEmitter::genUseAttrAsResultTypeCollectiveParamBuilder(
   // If the builder is redundant, skip generating the method
   if (!m)
     return;
-  genInlineCreateBody(paramList);
+  bool deprecated = kind == CollectiveBuilderKind::AttrDict &&
+                    emitHelper.hasNonEmptyPropertiesStruct();
+  if (deprecated)
+    m->setDeprecated(legacyBuilderDeprecation);
+  genInlineCreateBody(paramList, deprecated);
 
   auto &body = m->body();
 
@@ -2976,6 +3067,10 @@ void OpEmitter::genUseAttrAsResultTypeCollectiveParamBuilder(
   if (kind == CollectiveBuilderKind::PropStruct) {
     body << "  ::mlir::Attribute typeAttr = properties."
          << op.getGetterName(namedAttr.name) << "();\n";
+  } else if (emitHelper.hasNonEmptyPropertiesStruct()) {
+    genCodeForAddingPropertiesAndAttributes(body, kind, attributesName);
+    body << "  ::mlir::Attribute typeAttr = " << builderOpStateProperties << "."
+         << op.getGetterName(namedAttr.name) << "();\n";
   } else {
     body << "  ::mlir::Attribute typeAttr;\n"
          << "  auto attrName = " << op.getGetterName(namedAttr.name)
@@ -2992,14 +3087,9 @@ void OpEmitter::genUseAttrAsResultTypeCollectiveParamBuilder(
   // Operands
   body << "  " << builderOpState << ".addOperands(operands);\n";
 
-  // Properties
-  if (kind == CollectiveBuilderKind::PropStruct)
-    body << "  " << builderOpState
-         << ".useProperties(const_cast<Properties&>(properties));\n";
-
-  // Attributes
-  body << "  " << builderOpState << ".addAttributes(" << attributesName
-       << ");\n";
+  if (kind == CollectiveBuilderKind::PropStruct ||
+      !emitHelper.hasNonEmptyPropertiesStruct())
+    genCodeForAddingPropertiesAndAttributes(body, kind, attributesName);
 
   // Result types
   SmallVector<std::string, 2> resultTypes(op.getNumResults(), resultType);
@@ -3067,6 +3157,8 @@ void OpEmitter::genBuilder() {
   if (op.skipDefaultBuilders())
     return;
 
+  genLegacyPropertiesBuilderHelper();
+
   // We generate three classes of builders here:
   // 1. one having a stand-alone parameter for each operand / attribute, and
   genSeparateArgParamBuilder();
@@ -3126,7 +3218,11 @@ void OpEmitter::genCollectiveParamBuilder(CollectiveBuilderKind kind) {
   // If the builder is redundant, skip generating the method
   if (!m)
     return;
-  genInlineCreateBody(paramList);
+  bool deprecated = kind == CollectiveBuilderKind::AttrDict &&
+                    emitHelper.hasNonEmptyPropertiesStruct();
+  if (deprecated)
+    m->setDeprecated(legacyBuilderDeprecation);
+  genInlineCreateBody(paramList, deprecated);
   auto &body = m->body();
 
   // Operands
@@ -3137,14 +3233,7 @@ void OpEmitter::genCollectiveParamBuilder(CollectiveBuilderKind kind) {
          << "u && \"mismatched number of parameters\");\n";
   body << "  " << builderOpState << ".addOperands(operands);\n";
 
-  // Properties
-  if (kind == CollectiveBuilderKind::PropStruct)
-    body << "  " << builderOpState
-         << ".useProperties(const_cast<Properties&>(properties));\n";
-
-  // Attributes
-  body << "  " << builderOpState << ".addAttributes(" << attributesName
-       << ");\n";
+  genCodeForAddingPropertiesAndAttributes(body, kind, attributesName);
 
   // Create the correct number of regions
   if (int numRegions = op.getNumRegions()) {
@@ -3161,23 +3250,6 @@ void OpEmitter::genCollectiveParamBuilder(CollectiveBuilderKind kind) {
          << "u && \"mismatched number of return types\");\n";
   body << "  " << builderOpState << ".addTypes(resultTypes);\n";
 
-  if (emitHelper.hasNonEmptyPropertiesStruct() &&
-      kind == CollectiveBuilderKind::AttrDict) {
-    // Initialize the properties from Attributes before invoking the infer
-    // function.
-    body << formatv(R"(
-  if (!attributes.empty()) {
-    (void){1}.getOrAddProperties<{0}::Properties>();
-    ::mlir::PropertyRef properties = {1}.getRawProperties();
-    std::optional<::mlir::RegisteredOperationName> info =
-      {1}.name.getRegisteredInfo();
-    if (failed(info->setOpPropertiesFromAttribute({1}.name, properties,
-        {1}.attributes.getDictionary({1}.getContext()), nullptr)))
-      ::llvm::report_fatal_error("Property conversion failed.");
-  })",
-                    opClass.getClassName(), builderOpState);
-  }
-
   // Generate builder that infers type too.
   // TODO: Expand to handle successors.
   if (canInferType(op) && op.getNumSuccessors() == 0)
diff --git a/mlir/unittests/TableGen/OpBuildGen.cpp b/mlir/unittests/TableGen/OpBuildGen.cpp
index f34aa4af87e9f..9d69f0fd32068 100644
--- a/mlir/unittests/TableGen/OpBuildGen.cpp
+++ b/mlir/unittests/TableGen/OpBuildGen.cpp
@@ -16,6 +16,7 @@
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/BuiltinTypes.h"
 #include "mlir/IR/Dialect.h"
+#include "llvm/Support/Compiler.h"
 #include "gmock/gmock.h"
 #include <vector>
 
@@ -289,13 +290,17 @@ TEST_F(OpBuildGenTest, BuildMethodsVariadicProperties) {
   verifyOp(std::move(op), {f32Ty}, {*cstI32}, {*cstI32}, noAttrs);
 
   // Test build method with result types, supplied attributes.
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_PUSH
   op = test::TableGenBuildOp6::create(builder, loc, TypeRange{f32Ty},
                                       ValueRange{*cstI32, *cstI32}, attrs);
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_POP
   verifyOp(std::move(op), {f32Ty}, {*cstI32}, {*cstI32}, attrs);
 
   // Test build method with no result types and supplied attributes.
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_PUSH
   op = test::TableGenBuildOp6::create(builder, loc,
                                       ValueRange{*cstI32, *cstI32}, attrs);
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_POP
   verifyOp(std::move(op), {f32Ty}, {*cstI32}, {*cstI32}, attrs);
 
   // Test replacing an inherent attribute backed by a native property.
@@ -331,14 +336,120 @@ TEST_F(OpBuildGenTest, BuildMethodsInherentDiscardableAttrs) {
   replacedAttrs[0].setValue(replacement);
   verifyOp(op7, {}, {}, replacedAttrs);
 
-  // Check that the old-style builder where all the attributes go in the same
-  // place works.
+  // Check that the old-style builder partitions the attributes and populates
+  // properties before Operation::create.
+  OperationState state(loc, test::TableGenBuildOp7::getOperationName());
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_PUSH
+  test::TableGenBuildOp7::build(builder, state, TypeRange{}, ValueRange{},
+                                attrs);
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_POP
+  ASSERT_TRUE(state.getRawProperties());
+  EXPECT_EQ(state.attributes.getAttrs().size(), 1u);
+  EXPECT_EQ(state.attributes.getAttrs()[0], attrs[1]);
+  EXPECT_EQ(
+      state.getOrAddProperties<test::TableGenBuildOp7::Properties>().getAttr0(),
+      attrs[0].getValue());
+
+  auto op7FromState = cast<test::TableGenBuildOp7>(builder.create(state));
+  verifyOp(op7FromState, {}, {}, attrs);
+
+  // Check that the deprecated create forwarder remains compatible.
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_PUSH
   auto op7b = test::TableGenBuildOp7::create(builder, loc, TypeRange{},
                                              ValueRange{}, attrs);
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_POP
   // Note: this goes before verifyOp() because verifyOp() calls erase(), causing
   // use-after-free.
   ASSERT_EQ(op7b.getProperties().getAttr0(), attrs[0].getValue());
   verifyOp(op7b, {}, {}, attrs);
 }
 
+TEST_F(OpBuildGenTest, BuildMethodsLegacyMixedProperties) {
+  SmallVector<NamedAttribute> mixedAttrs{
+      builder.getNamedAttr("attr0", builder.getBoolAttr(true)),
+      builder.getNamedAttr("nativeProp", builder.getI64IntegerAttr(42)),
+      builder.getNamedAttr("operand_segment_sizes",
+                           builder.getDenseI32ArrayAttr({1, 1})),
+      builder.getNamedAttr("result_segment_sizes",
+                           builder.getDenseI32ArrayAttr({1, 0})),
+      builder.getNamedAttr("unknown", builder.getStringAttr("discardable"))};
+  OperationState state(loc, test::TableGenBuildOp8::getOperationName());
+
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_PUSH
+  test::TableGenBuildOp8::build(builder, state, ValueRange{*cstI32, *cstF32},
+                                mixedAttrs);
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_POP
+
+  ASSERT_TRUE(state.getRawProperties());
+  ASSERT_EQ(state.attributes.getAttrs().size(), 1u);
+  EXPECT_EQ(state.attributes.getAttrs()[0], mixedAttrs.back());
+  ASSERT_EQ(state.types.size(), 1u);
+  EXPECT_EQ(state.types[0], i32Ty);
+  const auto &properties =
+      state.getOrAddProperties<test::TableGenBuildOp8::Properties>();
+  EXPECT_TRUE(properties.attr0.getValue());
+  EXPECT_EQ(properties.defaultAttr.getInt(), 7);
+  EXPECT_EQ(properties.nativeProp, 42);
+  EXPECT_EQ(properties.operandSegmentSizes, (std::array<int32_t, 2>{1, 1}));
+  EXPECT_EQ(properties.resultSegmentSizes, (std::array<int32_t, 2>{1, 0}));
+
+  auto op = cast<test::TableGenBuildOp8>(builder.create(state));
+  EXPECT_EQ(op->getDiscardableAttrDictionary().size(), 1u);
+  EXPECT_EQ(op.getNativeProp(), 42);
+  EXPECT_EQ(op.getDefaultAttr(), 7u);
+  EXPECT_EQ(op->getResult(0).getType(), i32Ty);
+  EXPECT_TRUE(succeeded(op.verify()));
+  op.erase();
+}
+
+TEST_F(OpBuildGenTest, BuildMethodsLegacySameOperandAndResultType) {
+  SmallVector<NamedAttribute> mixedAttrs{
+      builder.getNamedAttr("attr0", builder.getBoolAttr(true)),
+      builder.getNamedAttr("unknown", builder.getUnitAttr())};
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_PUSH
+  auto op = test::TableGenBuildOp9::create(builder, loc, ValueRange{*cstI32},
+                                           mixedAttrs);
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_POP
+  EXPECT_EQ(op.getResult().getType(), i32Ty);
+  EXPECT_TRUE(op.getAttr0());
+  EXPECT_EQ(op->getDiscardableAttrDictionary().size(), 1u);
+  EXPECT_TRUE(succeeded(op.verify()));
+  op.erase();
+}
+
+TEST_F(OpBuildGenTest, BuildMethodsLegacyFirstAttrDerivedResultType) {
+  SmallVector<NamedAttribute> mixedAttrs{
+      builder.getNamedAttr("type", TypeAttr::get(f32Ty)),
+      builder.getNamedAttr("unknown", builder.getUnitAttr())};
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_PUSH
+  auto op = test::TableGenBuildOp10::create(builder, loc, ValueRange{*cstI32},
+                                            mixedAttrs);
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_POP
+  EXPECT_EQ(op.getResult().getType(), f32Ty);
+  EXPECT_EQ(op.getType(), f32Ty);
+  EXPECT_EQ(op->getDiscardableAttrDictionary().size(), 1u);
+  EXPECT_TRUE(succeeded(op.verify()));
+  op.erase();
+}
+
+TEST_F(OpBuildGenTest, BuildMethodsEmptyPropertiesKeepMixedAttributes) {
+  OperationState state(loc, test::TableGenBuildOp0::getOperationName());
+  test::TableGenBuildOp0::build(builder, state, TypeRange{i32Ty},
+                                ValueRange{*cstI32}, attrs);
+  EXPECT_FALSE(state.getRawProperties());
+  EXPECT_EQ(state.attributes.getAttrs(), attrs);
+}
+
+TEST_F(OpBuildGenTest, BuildMethodsInvalidLegacyPropertyConversion) {
+  SmallVector<NamedAttribute> badAttrs{
+      builder.getNamedAttr("attr0", builder.getStringAttr("not-a-bool"))};
+  OperationState state(loc, test::TableGenBuildOp7::getOperationName());
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_PUSH
+  EXPECT_DEATH_IF_SUPPORTED(
+      test::TableGenBuildOp7::build(builder, state, TypeRange{}, ValueRange{},
+                                    badAttrs),
+      "Invalid attribute.*attr0");
+  LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_POP
+}
+
 } // namespace mlir



More information about the Mlir-commits mailing list