Author: Benjamin Chetioui Date: 2025-03-06T16:26:44+01:00 New Revision: 1d72977e0198033e8ae7f7317abec09d59b330de
URL: https://github.com/llvm/llvm-project/commit/1d72977e0198033e8ae7f7317abec09d59b330de DIFF: https://github.com/llvm/llvm-project/commit/1d72977e0198033e8ae7f7317abec09d59b330de.diff LOG: Revert "[mlir][ODS] Add a generated builder that takes the Properties struct …" This reverts commit 35622a93bb034ad5c56e3a490060648b35ba49f1. Added: Modified: mlir/docs/DeclarativeRewrites.md mlir/docs/DefiningDialects/Operations.md mlir/include/mlir/IR/OpDefinition.h mlir/include/mlir/IR/OperationSupport.h mlir/test/lib/Dialect/Test/TestOps.td mlir/test/mlir-tblgen/op-attribute.td mlir/test/mlir-tblgen/op-decl-and-defs.td mlir/test/mlir-tblgen/op-result.td mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp mlir/unittests/TableGen/OpBuildGen.cpp Removed: ################################################################################ diff --git a/mlir/docs/DeclarativeRewrites.md b/mlir/docs/DeclarativeRewrites.md index fd566a2393b63..888ce57fa3b53 100644 --- a/mlir/docs/DeclarativeRewrites.md +++ b/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 diff --git a/mlir/docs/DefiningDialects/Operations.md b/mlir/docs/DefiningDialects/Operations.md index 528070cd3ebff..8ff60ac21424c 100644 --- a/mlir/docs/DefiningDialects/Operations.md +++ b/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 same form regardless of the exact op. This is particularly useful for +The first form provides 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. -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 diff erent +The third form will be generated if any of the op's attribute has diff erent `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 diff --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h index 4fad61580b31a..d91c573c03efe 100644 --- a/mlir/include/mlir/IR/OpDefinition.h +++ b/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 { - bool operator==(const EmptyProperties &) const { return true; } - bool operator!=(const EmptyProperties &) const { return false; } -}; +struct EmptyProperties {}; /// Traits to detect whether an Operation defined a `Properties` type, otherwise /// it'll default to `EmptyProperties`. diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h index 8376c7dbea1d3..2b0e50437afcc 100644 --- a/mlir/include/mlir/IR/OperationSupport.h +++ b/mlir/include/mlir/IR/OperationSupport.h @@ -1029,24 +1029,6 @@ struct OperationState { 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) { diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td index cc0fe924acf75..cdc1237ec8c5a 100644 --- a/mlir/test/lib/Dialect/Test/TestOps.td +++ b/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 //===----------------------------------------------------------------------===// diff --git a/mlir/test/mlir-tblgen/op-attribute.td b/mlir/test/mlir-tblgen/op-attribute.td index 549830e06042f..55382a5bd3f8c 100644 --- a/mlir/test/mlir-tblgen/op-attribute.td +++ b/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 diff --git a/mlir/test/mlir-tblgen/op-decl-and-defs.td b/mlir/test/mlir-tblgen/op-decl-and-defs.td index a3dce9b31f8d2..ee800a2952bac 100644 --- a/mlir/test/mlir-tblgen/op-decl-and-defs.td +++ b/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); diff --git a/mlir/test/mlir-tblgen/op-result.td b/mlir/test/mlir-tblgen/op-result.td index a4f7af6dbcf1c..212d3189cf571 100644 --- a/mlir/test/mlir-tblgen/op-result.td +++ b/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: 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()}); +// CHECK: odsState.addTypes({::llvm::cast<::mlir::TypeAttr>(attr.getValue()).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: 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()}); +// CHECK: odsState.addTypes({::llvm::cast<::mlir::TypedAttr>(attr.getValue()).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. // diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp index b957c8ee9f8ab..1970647a80115 100644 --- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp +++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp @@ -411,15 +411,6 @@ class OpOrAdaptorHelper { 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 @@ class OpEmitter { // 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 - genUseOperandAsResultTypeCollectiveParamBuilder(CollectiveBuilderKind kind); + void genUseOperandAsResultTypeCollectiveParamBuilder(); // 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 - // parameters. The generated build() method uses first attribute's + // Generates the build() method that takes each operand/attribute as a + // 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( - CollectiveBuilderKind kind) { +void OpEmitter::genUseOperandAsResultTypeCollectiveParamBuilder() { 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 - << ");\n"; + body << " " << builderOpState << ".addAttributes(attributes);\n"; // Create the correct number of regions if (int numRegions = op.getNumRegions()) { @@ -2781,20 +2752,14 @@ void OpEmitter::genPopulateDefaultAttributes() { } } -void OpEmitter::genInferredTypeCollectiveParamBuilder( - CollectiveBuilderKind kind) { +void OpEmitter::genInferredTypeCollectiveParamBuilder() { 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 - << ".useProperties(const_cast<Properties &>(properties));\n"; - body << " " << builderOpState << ".addAttributes(" << attributesName - << ");\n"; + body << " " << builderOpState << ".addAttributes(attributes);\n"; // Create the correct number of regions if (int numRegions = op.getNumRegions()) { @@ -2834,8 +2795,7 @@ void OpEmitter::genInferredTypeCollectiveParamBuilder( } // Result types - if (emitHelper.hasNonEmptyPropertiesStruct() && - kind == CollectiveBuilderKind::AttrDict) { + if (emitHelper.hasProperties()) { // 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( - CollectiveBuilderKind kind) { +void OpEmitter::genUseAttrAsResultTypeBuilder() { 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); + body << " auto attrName = " << op.getGetterName(namedAttr.name) + << "AttrName(" << builderOpState + << ".name);\n" + " for (auto attr : attributes) {\n" + " if (attr.getName() != attrName) continue;\n"; if (namedAttr.attr.isTypeAttr()) { - 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"; + resultType = "::llvm::cast<::mlir::TypeAttr>(attr.getValue()).getValue()"; } 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" - " typeAttr = attr.getValue();\n" - " break;\n" - " }\n" - " }\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 - << ");\n"; + body << " " << builderOpState << ".addAttributes(attributes);\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 - genCollectiveParamBuilder(CollectiveBuilderKind::AttrDict); - if (emitHelper.hasProperties()) - genCollectiveParamBuilder(CollectiveBuilderKind::PropStruct); + // attributes, and + genCollectiveParamBuilder(); // 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 diff erent call patterns. if (op.getNumVariableLengthResults() == 0) { if (op.getTrait("::mlir::OpTrait::SameOperandsAndResultType")) { genUseOperandAsResultTypeSeparateParamBuilder(); - genUseOperandAsResultTypeCollectiveParamBuilder( - CollectiveBuilderKind::AttrDict); - if (emitHelper.hasProperties()) - genUseOperandAsResultTypeCollectiveParamBuilder( - CollectiveBuilderKind::PropStruct); - } - if (op.getTrait("::mlir::OpTrait::FirstAttrDerivedResultType")) { - genUseAttrAsResultTypeCollectiveParamBuilder( - CollectiveBuilderKind::AttrDict); - genUseAttrAsResultTypeCollectiveParamBuilder( - CollectiveBuilderKind::PropStruct); + genUseOperandAsResultTypeCollectiveParamBuilder(); } + 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 - << ");\n"; + body << " " << builderOpState << ".addAttributes(attributes);\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() && - kind == CollectiveBuilderKind::AttrDict) { + if (emitHelper.hasProperties()) { // 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> ¶mList, @@ -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<UsingDeclaration>("Properties", - "::mlir::EmptyProperties"); - else - genericAdaptorBase.declare<ExtraClassDeclaration>( - std::move(declarations)); + genericAdaptorBase.declare<ExtraClassDeclaration>(std::move(declarations)); } genericAdaptorBase.declare<VisibilityDeclaration>(Visibility::Protected); genericAdaptorBase.declare<Field>("::mlir::DictionaryAttr", "odsAttrs"); diff --git a/mlir/unittests/TableGen/OpBuildGen.cpp b/mlir/unittests/TableGen/OpBuildGen.cpp index b4a5c316d63d6..94fbfa28803c4 100644 --- a/mlir/unittests/TableGen/OpBuildGen.cpp +++ b/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 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
