Revert "[mlir][ODS] Add a generated builder that takes the Properties struct" (#130117)

Reverts llvm/llvm-project#124713.

Builders involving sanitizers are failing:
https://lab.llvm.org/buildbot/#/builders/169/builds/9106.

mlir/docs/DeclarativeRewrites.md

@@ -237,9 +237,9 @@ In the above, we are using `BOp`'s result for building `COp`.
 
 Given that `COp` was specified with table-driven op definition, there will be
 several `build()` methods generated for it. One of them has aggregated
-parameters for result types, operands, and properties in the signature: `void
+parameters for result types, operands, and attributes in the signature: `void
 COp::build(..., ArrayRef<Type> resultTypes, Array<Value> operands,
-const COp::Properties& properties)`. The pattern in the above calls this `build()`
+ArrayRef<NamedAttribute> attr)`. The pattern in the above calls this `build()`
 method for constructing the `COp`.
 
 In general, arguments in the result pattern will be passed directly to the

mlir/docs/DefiningDialects/Operations.md

@@ -465,18 +465,7 @@ def MyOp : ... {
 The following builders are generated:
 
 ```c++
-// All result-types/operands/properties/discardable attributes have one
-// aggregate parameter. `Properties` is the properties structure of
-// `MyOp`.
-static void build(OpBuilder &odsBuilder, OperationState &odsState,
-                  TypeRange resultTypes,
-                  ValueRange operands,
-                  Properties properties,
-                  ArrayRef<NamedAttribute> discardableAttributes = {});
-
 // All result-types/operands/attributes have one aggregate parameter.
-// Inherent properties and discardable attributes are mixed together in the
-//  `attributes` dictionary.
 static void build(OpBuilder &odsBuilder, OperationState &odsState,
                   TypeRange resultTypes,
                   ValueRange operands,
@@ -509,28 +498,20 @@ static void build(OpBuilder &odsBuilder, OperationState &odsState,
 
 // All operands/attributes have aggregate parameters.
 // Generated if return type can be inferred.
-static void build(OpBuilder &odsBuilder, OperationState &odsState,
-                  ValueRange operands,
-                  Properties properties,
-                  ArrayRef<NamedAttribute> discardableAttributes);
-
-// All operands/attributes have aggregate parameters.
-// Generated if return type can be inferred. Uses the legacy merged attribute
-// dictionary.
 static void build(OpBuilder &odsBuilder, OperationState &odsState,
                   ValueRange operands, ArrayRef<NamedAttribute> attributes);
 
 // (And manually specified builders depending on the specific op.)
 ```
 
-The first two forms provide basic uniformity so that we can create ops using
+The first form provides basic uniformity so that we can create ops using the
-the same form regardless of the exact op. This is particularly useful for
+same form regardless of the exact op. This is particularly useful for
 implementing declarative pattern rewrites.
 
-The third and fourth forms are good for use in manually written code, given that
+The second and third forms are good for use in manually written code, given that
 they provide better guarantee via signatures.
 
-The fourth form will be generated if any of the op's attribute has different
+The third form will be generated if any of the op's attribute has different
 `Attr.returnType` from `Attr.storageType` and we know how to build an attribute
 from an unwrapped value (i.e., `Attr.constBuilderCall` is defined.)
 Additionally, for the third form, if an attribute appearing later in the

mlir/include/mlir/IR/OpDefinition.h

@@ -74,10 +74,7 @@ void ensureRegionTerminator(
 
 /// Structure used by default as a "marker" when no "Properties" are set on an
 /// Operation.
-struct EmptyProperties {
+struct EmptyProperties {};
-  bool operator==(const EmptyProperties &) const { return true; }
-  bool operator!=(const EmptyProperties &) const { return false; }
-};
 
 /// Traits to detect whether an Operation defined a `Properties` type, otherwise
 /// it'll default to `EmptyProperties`.

mlir/include/mlir/IR/OperationSupport.h

@@ -1029,24 +1029,6 @@ public:
   setProperties(Operation *op,
                 function_ref<InFlightDiagnostic()> emitError) const;
 
-  // Make `newProperties` the source of the properties that will be copied into
-  // the operation. The memory referenced by `newProperties` must remain live
-  // until after the `Operation` is created, at which time it may be
-  // deallocated. Calls to `getOrAddProperties<>() will return references to
-  // this memory.
-  template <typename T>
-  void useProperties(T &newProperties) {
-    assert(!properties &&
-           "Can't provide a properties struct when one has been allocated");
-    properties = &newProperties;
-    propertiesDeleter = [](OpaqueProperties) {};
-    propertiesSetter = [](OpaqueProperties newProp,
-                          const OpaqueProperties prop) {
-      *newProp.as<T *>() = *prop.as<const T *>();
-    };
-    propertiesId = TypeID::get<T>();
-  }
-
   void addOperands(ValueRange newOperands);
 
   void addTypes(ArrayRef<Type> newTypes) {

mlir/test/lib/Dialect/Test/TestOps.td

@@ -2504,13 +2504,6 @@ def TableGenBuildOp6 : TEST_Op<"tblgen_build_6", [AttrSizedOperandSegments]> {
   let results = (outs F32:$result);
 }
 
-// An inherent attribute. Test collective builders, both those that take properties as
-// properties structs and those that take an attribute dictionary.
-def TableGenBuildOp7 : TEST_Op<"tblgen_build_7", []> {
-  let arguments = (ins BoolAttr:$attr0);
-  let results = (outs);
-}
-
 //===----------------------------------------------------------------------===//
 // Test BufferPlacement
 //===----------------------------------------------------------------------===//

mlir/test/mlir-tblgen/op-attribute.td

@@ -165,12 +165,6 @@ def AOp : NS_Op<"a_op", []> {
 // DEF:        ::llvm::ArrayRef<::mlir::NamedAttribute> attributes
 // DEF:      odsState.addAttributes(attributes);
 
-// DEF:      void AOp::build(
-// DEF-SAME:   const Properties &properties,
-// DEF-SAME:   ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes
-// DEF:      odsState.useProperties(const_cast<Properties&>(properties));
-// DEF:      odsState.addAttributes(discardableAttributes);
-
 // DEF:      void AOp::populateDefaultProperties
 
 // Test the above but with prefix.
@@ -285,12 +279,6 @@ def AgetOp : Op<Test2_Dialect, "a_get_op", []> {
 // DEF:        ::llvm::ArrayRef<::mlir::NamedAttribute> attributes
 // DEF:      odsState.addAttributes(attributes);
 
-// DEF:      void AgetOp::build(
-// DEF-SAME:   const Properties &properties
-// DEF-SAME:   ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes
-// DEF:      odsState.useProperties(const_cast<Properties&>(properties));
-// DEF:      odsState.addAttributes(discardableAttributes);
-
 // Test the above but using properties.
 def ApropOp : NS_Op<"a_prop_op", []> {
   let arguments = (ins

mlir/test/mlir-tblgen/op-decl-and-defs.td

@@ -119,7 +119,6 @@ def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> {
 // CHECK:   static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::Type r, ::mlir::TypeRange s, ::mlir::Value a, ::mlir::ValueRange b, uint32_t attr1, /*optional*/::mlir::FloatAttr some_attr2, unsigned someRegionsCount)
 // 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 void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::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 ::mlir::ParseResult parse(::mlir::OpAsmParser &parser, ::mlir::OperationState &result);
 // CHECK:   void print(::mlir::OpAsmPrinter &p);
 // CHECK:   ::llvm::LogicalResult verifyInvariants();
@@ -232,7 +231,6 @@ def NS_HCollectiveParamsOp : NS_Op<"op_collective_params", []> {
 // CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::Type b, ::mlir::Value a);
 // CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::Value a);
 // CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::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 suppression of "separate arg, separate result" build method for an op
 // with single variadic arg and single variadic result (since it will be
@@ -283,8 +281,6 @@ def NS_IOp : NS_Op<"op_with_same_operands_and_result_types_trait", [SameOperands
 // CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::Value a, ::mlir::Value b);
 // CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::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 void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::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 default value of `attributes` for the `genInferredTypeCollectiveParamBuilder` builder
 def NS_JOp : NS_Op<"op_with_InferTypeOpInterface_interface", [DeclareOpInterfaceMethods<InferTypeOpInterface>]> {
@@ -297,8 +293,6 @@ def NS_JOp : NS_Op<"op_with_InferTypeOpInterface_interface", [DeclareOpInterface
 // CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::Value a, ::mlir::Value b);
 // CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::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 void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::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 = {});
 
 // Test usage of TraitList getting flattened during emission.
 def NS_KOp : NS_Op<"k_op", [IsolatedFromAbove,
@@ -335,8 +329,6 @@ def NS_LOp : NS_Op<"op_with_same_operands_and_result_types_unwrapped_attr", [Sam
 // CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::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 void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::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 void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::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);

mlir/test/mlir-tblgen/op-result.td

@@ -57,9 +57,7 @@ def OpD : NS_Op<"type_attr_as_result_type", [FirstAttrDerivedResultType]> {
 
 // CHECK-LABEL: OpD definitions
 // CHECK: void OpD::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes)
-// CHECK: odsState.addTypes({::llvm::cast<::mlir::TypeAttr>(typeAttr).getValue()});
+// CHECK: odsState.addTypes({::llvm::cast<::mlir::TypeAttr>(attr.getValue()).getValue()});
-// CHECK: void OpD::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes)
-// CHECK: odsState.addTypes({::llvm::cast<::mlir::TypeAttr>(typeAttr).getValue()});
 
 def OpE : NS_Op<"value_attr_as_result_type", [FirstAttrDerivedResultType]> {
   let arguments = (ins I32:$x, F32Attr:$attr);
@@ -68,10 +66,7 @@ def OpE : NS_Op<"value_attr_as_result_type", [FirstAttrDerivedResultType]> {
 
 // CHECK-LABEL: OpE definitions
 // CHECK: void OpE::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes)
-// CHECK: odsState.addTypes({::llvm::cast<::mlir::TypedAttr>(typeAttr).getType()});
+// CHECK: odsState.addTypes({::llvm::cast<::mlir::TypedAttr>(attr.getValue()).getType()});
-// CHECK: void OpE::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes)
-// CHECK: ::mlir::Attribute typeAttr = properties.getAttr();
-// CHECK: odsState.addTypes({::llvm::cast<::mlir::TypedAttr>(typeAttr).getType()});
 
 def OpF : NS_Op<"one_variadic_result_op", []> {
   let results = (outs Variadic<I32>:$x);
@@ -123,8 +118,6 @@ def OpK : NS_Op<"only_input_is_variadic_with_same_value_type_op", [SameOperandsA
 
 // CHECK-LABEL: OpK::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes)
 // CHECK: odsState.addTypes({operands[0].getType()});
-// CHECK-LABEL: OpK::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes)
-// CHECK: odsState.addTypes({operands[0].getType()});
 
 // Test with inferred shapes and interleaved with operands/attributes.
 //

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

@@ -411,15 +411,6 @@ public:
       return true;
     if (!op.getDialect().usePropertiesForAttributes())
       return false;
-    return true;
-  }
-
-  /// Returns whether the operation will have a non-empty `Properties` struct.
-  bool hasNonEmptyPropertiesStruct() const {
-    if (!op.getProperties().empty())
-      return true;
-    if (!hasProperties())
-      return false;
     if (op.getTrait("::mlir::OpTrait::AttrSizedOperandSegments") ||
         op.getTrait("::mlir::OpTrait::AttrSizedResultSegments"))
       return true;
@@ -670,33 +661,24 @@ private:
   // type as all results' types.
   void genUseOperandAsResultTypeSeparateParamBuilder();
 
-  // The kind of collective builder to generate
-  enum class CollectiveBuilderKind {
-    PropStruct, // Inherent attributes/properties are passed by `const
-                // Properties&`
-    AttrDict,   // Inherent attributes/properties are passed by attribute
-                // dictionary
-  };
-
   // 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.
-  void
+  void genUseOperandAsResultTypeCollectiveParamBuilder();
-  genUseOperandAsResultTypeCollectiveParamBuilder(CollectiveBuilderKind kind);
 
   // Generates the build() method that takes aggregate operands/attributes
   // parameters. This build() method uses inferred types as result types.
   // Requires: The type needs to be inferable via InferTypeOpInterface.
-  void genInferredTypeCollectiveParamBuilder(CollectiveBuilderKind kind);
+  void genInferredTypeCollectiveParamBuilder();
 
-  // Generates the build() method that takesaggregate operands/attributes as
+  // Generates the build() method that takes each operand/attribute as a
-  // parameters. The generated build() method uses first attribute's
+  // stand-alone parameter. The generated build() method uses first attribute's
   // type as all result's types.
-  void genUseAttrAsResultTypeCollectiveParamBuilder(CollectiveBuilderKind kind);
+  void genUseAttrAsResultTypeBuilder();
 
   // Generates the build() method that takes all result types collectively as
   // one parameter. Similarly for operands and attributes.
-  void genCollectiveParamBuilder(CollectiveBuilderKind kind);
+  void genCollectiveParamBuilder();
 
   // The kind of parameter to generate for result types in builders.
   enum class TypeParamKind {
@@ -1381,6 +1363,8 @@ void OpEmitter::genPropertiesSupport() {
     attrOrProperties.push_back(&emitHelper.getOperandSegmentsSize().value());
   if (emitHelper.getResultSegmentsSize())
     attrOrProperties.push_back(&emitHelper.getResultSegmentsSize().value());
+  if (attrOrProperties.empty())
+    return;
   auto &setPropMethod =
       opClass
           .addStaticMethod(
@@ -1744,9 +1728,6 @@ void OpEmitter::genPropertiesSupport() {
 
 void OpEmitter::genPropertiesSupportForBytecode(
     ArrayRef<ConstArgument> attrOrProperties) {
-  if (attrOrProperties.empty())
-    return;
-
   if (op.useCustomPropertiesEncoding()) {
     opClass.declareStaticMethod(
         "::llvm::LogicalResult", "readProperties",
@@ -2663,8 +2644,7 @@ void OpEmitter::genSeparateArgParamBuilder() {
   }
 }
 
-void OpEmitter::genUseOperandAsResultTypeCollectiveParamBuilder(
+void OpEmitter::genUseOperandAsResultTypeCollectiveParamBuilder() {
-    CollectiveBuilderKind kind) {
   int numResults = op.getNumResults();
 
   // Signature
@@ -2672,15 +2652,10 @@ void OpEmitter::genUseOperandAsResultTypeCollectiveParamBuilder(
   paramList.emplace_back("::mlir::OpBuilder &", "odsBuilder");
   paramList.emplace_back("::mlir::OperationState &", builderOpState);
   paramList.emplace_back("::mlir::ValueRange", "operands");
-  if (kind == CollectiveBuilderKind::PropStruct)
-    paramList.emplace_back("const Properties &", "properties");
   // Provide default value for `attributes` when its the last parameter
   StringRef attributesDefaultValue = op.getNumVariadicRegions() ? "" : "{}";
-  StringRef attributesName = kind == CollectiveBuilderKind::PropStruct
-                                 ? "discardableAttributes"
-                                 : "attributes";
   paramList.emplace_back("::llvm::ArrayRef<::mlir::NamedAttribute>",
-                         attributesName, attributesDefaultValue);
+                         "attributes", attributesDefaultValue);
   if (op.getNumVariadicRegions())
     paramList.emplace_back("unsigned", "numRegions");
 
@@ -2693,12 +2668,8 @@ void OpEmitter::genUseOperandAsResultTypeCollectiveParamBuilder(
   // Operands
   body << "  " << builderOpState << ".addOperands(operands);\n";
 
-  if (kind == CollectiveBuilderKind::PropStruct)
-    body << "  " << builderOpState
-         << ".useProperties(const_cast<Properties&>(properties));\n";
   // Attributes
-  body << "  " << builderOpState << ".addAttributes(" << attributesName
+  body << "  " << builderOpState << ".addAttributes(attributes);\n";
-       << ");\n";
 
   // Create the correct number of regions
   if (int numRegions = op.getNumRegions()) {
@@ -2781,20 +2752,14 @@ void OpEmitter::genPopulateDefaultAttributes() {
   }
 }
 
-void OpEmitter::genInferredTypeCollectiveParamBuilder(
+void OpEmitter::genInferredTypeCollectiveParamBuilder() {
-    CollectiveBuilderKind kind) {
   SmallVector<MethodParameter> paramList;
   paramList.emplace_back("::mlir::OpBuilder &", "odsBuilder");
   paramList.emplace_back("::mlir::OperationState &", builderOpState);
   paramList.emplace_back("::mlir::ValueRange", "operands");
-  if (kind == CollectiveBuilderKind::PropStruct)
-    paramList.emplace_back("const Properties &", "properties");
   StringRef attributesDefaultValue = op.getNumVariadicRegions() ? "" : "{}";
-  StringRef attributesName = kind == CollectiveBuilderKind::PropStruct
-                                 ? "discardableAttributes"
-                                 : "attributes";
   paramList.emplace_back("::llvm::ArrayRef<::mlir::NamedAttribute>",
-                         attributesName, attributesDefaultValue);
+                         "attributes", attributesDefaultValue);
   if (op.getNumVariadicRegions())
     paramList.emplace_back("unsigned", "numRegions");
 
@@ -2819,11 +2784,7 @@ void OpEmitter::genInferredTypeCollectiveParamBuilder(
          << numNonVariadicOperands
          << "u && \"mismatched number of parameters\");\n";
   body << "  " << builderOpState << ".addOperands(operands);\n";
-  if (kind == CollectiveBuilderKind::PropStruct)
+  body << "  " << builderOpState << ".addAttributes(attributes);\n";
-    body << "  " << builderOpState
-         << ".useProperties(const_cast<Properties &>(properties));\n";
-  body << "  " << builderOpState << ".addAttributes(" << attributesName
-       << ");\n";
 
   // Create the correct number of regions
   if (int numRegions = op.getNumRegions()) {
@@ -2834,8 +2795,7 @@ void OpEmitter::genInferredTypeCollectiveParamBuilder(
   }
 
   // Result types
-  if (emitHelper.hasNonEmptyPropertiesStruct() &&
+  if (emitHelper.hasProperties()) {
-      kind == CollectiveBuilderKind::AttrDict) {
     // Initialize the properties from Attributes before invoking the infer
     // function.
     body << formatv(R"(
@@ -2907,19 +2867,13 @@ void OpEmitter::genUseOperandAsResultTypeSeparateParamBuilder() {
     emit(AttrParamKind::UnwrappedValue);
 }
 
-void OpEmitter::genUseAttrAsResultTypeCollectiveParamBuilder(
+void OpEmitter::genUseAttrAsResultTypeBuilder() {
-    CollectiveBuilderKind kind) {
   SmallVector<MethodParameter> paramList;
   paramList.emplace_back("::mlir::OpBuilder &", "odsBuilder");
   paramList.emplace_back("::mlir::OperationState &", builderOpState);
   paramList.emplace_back("::mlir::ValueRange", "operands");
-  if (kind == CollectiveBuilderKind::PropStruct)
-    paramList.emplace_back("const Properties &", "properties");
-  StringRef attributesName = kind == CollectiveBuilderKind::PropStruct
-                                 ? "discardableAttributes"
-                                 : "attributes";
   paramList.emplace_back("::llvm::ArrayRef<::mlir::NamedAttribute>",
-                         attributesName, "{}");
+                         "attributes", "{}");
   auto *m = opClass.addStaticMethod("void", "build", std::move(paramList));
   // If the builder is redundant, skip generating the method
   if (!m)
@@ -2931,44 +2885,28 @@ void OpEmitter::genUseAttrAsResultTypeCollectiveParamBuilder(
   std::string resultType;
   const auto &namedAttr = op.getAttribute(0);
 
-  if (namedAttr.attr.isTypeAttr()) {
+  body << "  auto attrName = " << op.getGetterName(namedAttr.name)
-    resultType = "::llvm::cast<::mlir::TypeAttr>(typeAttr).getValue()";
-  } else {
-    resultType = "::llvm::cast<::mlir::TypedAttr>(typeAttr).getType()";
-  }
-
-  if (kind == CollectiveBuilderKind::PropStruct) {
-    body << "  ::mlir::Attribute typeAttr = properties."
-         << op.getGetterName(namedAttr.name) << "();\n";
-  } else {
-    body << "  ::mlir::Attribute typeAttr;\n"
-         << "  auto attrName = " << op.getGetterName(namedAttr.name)
        << "AttrName(" << builderOpState
        << ".name);\n"
           "  for (auto attr : attributes) {\n"
-            "    if (attr.getName() == attrName) {\n"
+          "    if (attr.getName() != attrName) continue;\n";
-            "      typeAttr = attr.getValue();\n"
+  if (namedAttr.attr.isTypeAttr()) {
-            "      break;\n"
+    resultType = "::llvm::cast<::mlir::TypeAttr>(attr.getValue()).getValue()";
-            "    }\n"
+  } else {
-            "  }\n";
+    resultType = "::llvm::cast<::mlir::TypedAttr>(attr.getValue()).getType()";
   }
 
   // Operands
   body << "  " << builderOpState << ".addOperands(operands);\n";
 
-  // Properties
-  if (kind == CollectiveBuilderKind::PropStruct)
-    body << "  " << builderOpState
-         << ".useProperties(const_cast<Properties&>(properties));\n";
-
   // Attributes
-  body << "  " << builderOpState << ".addAttributes(" << attributesName
+  body << "  " << builderOpState << ".addAttributes(attributes);\n";
-       << ");\n";
 
   // Result types
   SmallVector<std::string, 2> resultTypes(op.getNumResults(), resultType);
   body << "    " << builderOpState << ".addTypes({"
        << llvm::join(resultTypes, ", ") << "});\n";
+  body << "  }\n";
 }
 
 /// Returns a signature of the builder. Updates the context `fctx` to enable
@@ -3035,32 +2973,22 @@ void OpEmitter::genBuilder() {
   // 1. one having a stand-alone parameter for each operand / attribute, and
   genSeparateArgParamBuilder();
   // 2. one having an aggregated parameter for all result types / operands /
-  //    [properties / discardable] attributes, and
+  //    attributes, and
-  genCollectiveParamBuilder(CollectiveBuilderKind::AttrDict);
+  genCollectiveParamBuilder();
-  if (emitHelper.hasProperties())
-    genCollectiveParamBuilder(CollectiveBuilderKind::PropStruct);
   // 3. one having a stand-alone parameter for each operand and attribute,
   //    use the first operand or attribute's type as all result types
   //    to facilitate different call patterns.
   if (op.getNumVariableLengthResults() == 0) {
     if (op.getTrait("::mlir::OpTrait::SameOperandsAndResultType")) {
       genUseOperandAsResultTypeSeparateParamBuilder();
-      genUseOperandAsResultTypeCollectiveParamBuilder(
+      genUseOperandAsResultTypeCollectiveParamBuilder();
-          CollectiveBuilderKind::AttrDict);
-      if (emitHelper.hasProperties())
-        genUseOperandAsResultTypeCollectiveParamBuilder(
-            CollectiveBuilderKind::PropStruct);
-    }
-    if (op.getTrait("::mlir::OpTrait::FirstAttrDerivedResultType")) {
-      genUseAttrAsResultTypeCollectiveParamBuilder(
-          CollectiveBuilderKind::AttrDict);
-      genUseAttrAsResultTypeCollectiveParamBuilder(
-          CollectiveBuilderKind::PropStruct);
     }
+    if (op.getTrait("::mlir::OpTrait::FirstAttrDerivedResultType"))
+      genUseAttrAsResultTypeBuilder();
   }
 }
 
-void OpEmitter::genCollectiveParamBuilder(CollectiveBuilderKind kind) {
+void OpEmitter::genCollectiveParamBuilder() {
   int numResults = op.getNumResults();
   int numVariadicResults = op.getNumVariableLengthResults();
   int numNonVariadicResults = numResults - numVariadicResults;
@@ -3074,15 +3002,10 @@ void OpEmitter::genCollectiveParamBuilder(CollectiveBuilderKind kind) {
   paramList.emplace_back("::mlir::OperationState &", builderOpState);
   paramList.emplace_back("::mlir::TypeRange", "resultTypes");
   paramList.emplace_back("::mlir::ValueRange", "operands");
-  if (kind == CollectiveBuilderKind::PropStruct)
-    paramList.emplace_back("const Properties &", "properties");
   // Provide default value for `attributes` when its the last parameter
   StringRef attributesDefaultValue = op.getNumVariadicRegions() ? "" : "{}";
-  StringRef attributesName = kind == CollectiveBuilderKind::PropStruct
-                                 ? "discardableAttributes"
-                                 : "attributes";
   paramList.emplace_back("::llvm::ArrayRef<::mlir::NamedAttribute>",
-                         attributesName, attributesDefaultValue);
+                         "attributes", attributesDefaultValue);
   if (op.getNumVariadicRegions())
     paramList.emplace_back("unsigned", "numRegions");
 
@@ -3100,14 +3023,8 @@ 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
+  body << "  " << builderOpState << ".addAttributes(attributes);\n";
-       << ");\n";
 
   // Create the correct number of regions
   if (int numRegions = op.getNumRegions()) {
@@ -3124,8 +3041,7 @@ void OpEmitter::genCollectiveParamBuilder(CollectiveBuilderKind kind) {
          << "u && \"mismatched number of return types\");\n";
   body << "  " << builderOpState << ".addTypes(resultTypes);\n";
 
-  if (emitHelper.hasNonEmptyPropertiesStruct() &&
+  if (emitHelper.hasProperties()) {
-      kind == CollectiveBuilderKind::AttrDict) {
     // Initialize the properties from Attributes before invoking the infer
     // function.
     body << formatv(R"(
@@ -3144,7 +3060,7 @@ void OpEmitter::genCollectiveParamBuilder(CollectiveBuilderKind kind) {
   // Generate builder that infers type too.
   // TODO: Expand to handle successors.
   if (canInferType(op) && op.getNumSuccessors() == 0)
-    genInferredTypeCollectiveParamBuilder(kind);
+    genInferredTypeCollectiveParamBuilder();
 }
 
 void OpEmitter::buildParamList(SmallVectorImpl<MethodParameter> &paramList,
@@ -4145,7 +4061,7 @@ void OpEmitter::genTraits() {
   // native/interface traits and after all the traits with `StructuralOpTrait`.
   opClass.addTrait("::mlir::OpTrait::OpInvariants");
 
-  if (emitHelper.hasNonEmptyPropertiesStruct())
+  if (emitHelper.hasProperties())
     opClass.addTrait("::mlir::BytecodeOpInterface::Trait");
 
   // Add the native and interface traits.
@@ -4285,6 +4201,7 @@ OpOperandAdaptorEmitter::OpOperandAdaptorEmitter(
       attrOrProperties.push_back(&emitHelper.getOperandSegmentsSize().value());
     if (emitHelper.getResultSegmentsSize())
       attrOrProperties.push_back(&emitHelper.getResultSegmentsSize().value());
+    assert(!attrOrProperties.empty());
     std::string declarations = "  struct Properties {\n";
     llvm::raw_string_ostream os(declarations);
     std::string comparator =
@@ -4357,7 +4274,7 @@ OpOperandAdaptorEmitter::OpOperandAdaptorEmitter(
       // Emit accessors using the interface type.
       if (attr) {
         const char *accessorFmt = R"decl(
-    auto get{0}() const {
+    auto get{0}() {
       auto &propStorage = this->{1};
       return ::llvm::{2}<{3}>(propStorage);
     }
@@ -4379,12 +4296,7 @@ OpOperandAdaptorEmitter::OpOperandAdaptorEmitter(
     os << comparator;
     os << "  };\n";
 
-    if (attrOrProperties.empty())
+    genericAdaptorBase.declare<ExtraClassDeclaration>(std::move(declarations));
-      genericAdaptorBase.declare<UsingDeclaration>("Properties",
-                                                   "::mlir::EmptyProperties");
-    else
-      genericAdaptorBase.declare<ExtraClassDeclaration>(
-          std::move(declarations));
   }
   genericAdaptorBase.declare<VisibilityDeclaration>(Visibility::Protected);
   genericAdaptorBase.declare<Field>("::mlir::DictionaryAttr", "odsAttrs");

mlir/unittests/TableGen/OpBuildGen.cpp

@@ -291,20 +291,4 @@ TEST_F(OpBuildGenTest, BuildMethodsVariadicProperties) {
   verifyOp(std::move(op), {f32Ty}, {*cstI32}, {*cstI32}, attrs);
 }
 
-TEST_F(OpBuildGenTest, BuildMethodsInherentDiscardableAttrs) {
-  test::TableGenBuildOp7::Properties props;
-  props.attr0 = cast<BoolAttr>(attrs[0].getValue());
-  ArrayRef<NamedAttribute> discardableAttrs = attrs.drop_front();
-  auto op7 = builder.create<test::TableGenBuildOp7>(
-      loc, TypeRange{}, ValueRange{}, props, discardableAttrs);
-  verifyOp(op7, {}, {}, attrs);
-
-  // Check that the old-style builder where all the attributes go in the same
-  // place works.
-  auto op7b = builder.create<test::TableGenBuildOp7>(loc, TypeRange{},
-                                                     ValueRange{}, attrs);
-  verifyOp(op7b, {}, {}, attrs);
-  ASSERT_EQ(op7b.getProperties().getAttr0(), attrs[0].getValue());
-}
-
 } // namespace mlir