[Mlir-commits] [mlir] Revert "[mlir][ODS] Add a generated builder that takes the Properties struct" (PR #130117)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Thu Mar 6 07:27:59 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-core
Author: Benjamin Chetioui (bchetioui)
<details>
<summary>Changes</summary>
Reverts llvm/llvm-project#<!-- -->124713.
Builders involving sanitizers are failing: https://lab.llvm.org/buildbot/#/builders/169/builds/9106.
---
Patch is 34.28 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/130117.diff
10 Files Affected:
- (modified) mlir/docs/DeclarativeRewrites.md (+2-2)
- (modified) mlir/docs/DefiningDialects/Operations.md (+4-23)
- (modified) mlir/include/mlir/IR/OpDefinition.h (+1-4)
- (modified) mlir/include/mlir/IR/OperationSupport.h (-18)
- (modified) mlir/test/lib/Dialect/Test/TestOps.td (-7)
- (modified) mlir/test/mlir-tblgen/op-attribute.td (-12)
- (modified) mlir/test/mlir-tblgen/op-decl-and-defs.td (-8)
- (modified) mlir/test/mlir-tblgen/op-result.td (+2-9)
- (modified) mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp (+40-128)
- (modified) mlir/unittests/TableGen/OpBuildGen.cpp (-16)
``````````diff
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 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
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::genUseOperandAsResultTypeCollecti...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/130117
More information about the Mlir-commits
mailing list