Re-land "[mlir][ODS] Add a generated builder that takes the Properties struct" (#130117) #130373
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s struct" (llvm#130117) This reverts commit 32f5437. Investigations showed that the unit test utilities were calling erase(), causing a use-after-free. Problem has been fixed.
Member
|
@llvm/pr-subscribers-mlir Author: Krzysztof Drewniak (krzysz00) ChangesThis reverts commit 32f5437. Investigations showed that the unit test utilities were calling erase(), causing a use-after-free. Fixed by rearranging checks in the test Patch is 34.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/130373.diff 10 Files Affected:
diff --git a/mlir/docs/DeclarativeRewrites.md b/mlir/docs/DeclarativeRewrites.md
index 888ce57fa3b53..fd566a2393b63 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 attributes in the signature: `void
+parameters for result types, operands, and properties in the signature: `void
COp::build(..., ArrayRef<Type> resultTypes, Array<Value> operands,
-ArrayRef<NamedAttribute> attr)`. The pattern in the above calls this `build()`
+const COp::Properties& properties)`. 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 8ff60ac21424c..528070cd3ebff 100644
--- a/mlir/docs/DefiningDialects/Operations.md
+++ b/mlir/docs/DefiningDialects/Operations.md
@@ -465,7 +465,18 @@ 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,
@@ -498,20 +509,28 @@ 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 form provides basic uniformity so that we can create ops using the
-same form regardless of the exact op. This is particularly useful for
+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.
-The second and third forms are good for use in manually written code, given that
+The third and fourth forms are good for use in manually written code, given that
they provide better guarantee via signatures.
-The third form will be generated if any of the op's attribute has different
+The fourth 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 d91c573c03efe..4fad61580b31a 100644
--- a/mlir/include/mlir/IR/OpDefinition.h
+++ b/mlir/include/mlir/IR/OpDefinition.h
@@ -74,7 +74,10 @@ 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`.
diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 2b0e50437afcc..2d9fb2bc5859e 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -1029,6 +1029,36 @@ 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;
+#if defined(__clang__)
+#if __has_warning("-Wdangling-assignment-gsl")
+#pragma clang diagnostic push
+// https://github.com/llvm/llvm-project/issues/126600
+#pragma clang diagnostic ignored "-Wdangling-assignment-gsl"
+#endif
+#endif
+ propertiesDeleter = [](OpaqueProperties) {};
+ propertiesSetter = [](OpaqueProperties newProp,
+ const OpaqueProperties prop) {
+ *newProp.as<T *>() = *prop.as<const T *>();
+ };
+#if defined(__clang__)
+#if __has_warning("-Wdangling-assignment-gsl")
+#pragma clang diagnostic pop
+#endif
+#endif
+ 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 cdc1237ec8c5a..cc0fe924acf75 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -2504,6 +2504,13 @@ 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 55382a5bd3f8c..549830e06042f 100644
--- a/mlir/test/mlir-tblgen/op-attribute.td
+++ b/mlir/test/mlir-tblgen/op-attribute.td
@@ -165,6 +165,12 @@ 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.
@@ -279,6 +285,12 @@ 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 ee800a2952bac..a3dce9b31f8d2 100644
--- a/mlir/test/mlir-tblgen/op-decl-and-defs.td
+++ b/mlir/test/mlir-tblgen/op-decl-and-defs.td
@@ -119,6 +119,7 @@ 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();
@@ -231,6 +232,7 @@ 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
@@ -281,6 +283,8 @@ 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>]> {
@@ -293,6 +297,8 @@ 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,
@@ -329,6 +335,8 @@ 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 212d3189cf571..a4f7af6dbcf1c 100644
--- a/mlir/test/mlir-tblgen/op-result.td
+++ b/mlir/test/mlir-tblgen/op-result.td
@@ -57,7 +57,9 @@ 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>(attr.getValue()).getValue()});
+// 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()});
def OpE : NS_Op<"value_attr_as_result_type", [FirstAttrDerivedResultType]> {
let arguments = (ins I32:$x, F32Attr:$attr);
@@ -66,7 +68,10 @@ 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>(attr.getValue()).getType()});
+// 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()});
def OpF : NS_Op<"one_variadic_result_op", []> {
let results = (outs Variadic<I32>:$x);
@@ -118,6 +123,8 @@ 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 1970647a80115..b957c8ee9f8ab 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -411,6 +411,15 @@ 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;
@@ -661,24 +670,33 @@ 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();
+ void
+ 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();
+ void genInferredTypeCollectiveParamBuilder(CollectiveBuilderKind kind);
- // Generates the build() method that takes each operand/attribute as a
- // stand-alone parameter. The generated build() method uses first attribute's
+ // Generates the build() method that takesaggregate operands/attributes as
+ // parameters. The generated build() method uses first attribute's
// type as all result's types.
- void genUseAttrAsResultTypeBuilder();
+ void genUseAttrAsResultTypeCollectiveParamBuilder(CollectiveBuilderKind kind);
// Generates the build() method that takes all result types collectively as
// one parameter. Similarly for operands and attributes.
- void genCollectiveParamBuilder();
+ void genCollectiveParamBuilder(CollectiveBuilderKind kind);
// The kind of parameter to generate for result types in builders.
enum class TypeParamKind {
@@ -1363,8 +1381,6 @@ 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(
@@ -1728,6 +1744,9 @@ void OpEmitter::genPropertiesSupport() {
void OpEmitter::genPropertiesSupportForBytecode(
...
[truncated]
|
Member
|
@llvm/pr-subscribers-mlir-core Author: Krzysztof Drewniak (krzysz00) ChangesThis reverts commit 32f5437. Investigations showed that the unit test utilities were calling erase(), causing a use-after-free. Fixed by rearranging checks in the test Patch is 34.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/130373.diff 10 Files Affected:
diff --git a/mlir/docs/DeclarativeRewrites.md b/mlir/docs/DeclarativeRewrites.md
index 888ce57fa3b53..fd566a2393b63 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 attributes in the signature: `void
+parameters for result types, operands, and properties in the signature: `void
COp::build(..., ArrayRef<Type> resultTypes, Array<Value> operands,
-ArrayRef<NamedAttribute> attr)`. The pattern in the above calls this `build()`
+const COp::Properties& properties)`. 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 8ff60ac21424c..528070cd3ebff 100644
--- a/mlir/docs/DefiningDialects/Operations.md
+++ b/mlir/docs/DefiningDialects/Operations.md
@@ -465,7 +465,18 @@ 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,
@@ -498,20 +509,28 @@ 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 form provides basic uniformity so that we can create ops using the
-same form regardless of the exact op. This is particularly useful for
+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.
-The second and third forms are good for use in manually written code, given that
+The third and fourth forms are good for use in manually written code, given that
they provide better guarantee via signatures.
-The third form will be generated if any of the op's attribute has different
+The fourth 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 d91c573c03efe..4fad61580b31a 100644
--- a/mlir/include/mlir/IR/OpDefinition.h
+++ b/mlir/include/mlir/IR/OpDefinition.h
@@ -74,7 +74,10 @@ 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`.
diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 2b0e50437afcc..2d9fb2bc5859e 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -1029,6 +1029,36 @@ 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;
+#if defined(__clang__)
+#if __has_warning("-Wdangling-assignment-gsl")
+#pragma clang diagnostic push
+// https://github.com/llvm/llvm-project/issues/126600
+#pragma clang diagnostic ignored "-Wdangling-assignment-gsl"
+#endif
+#endif
+ propertiesDeleter = [](OpaqueProperties) {};
+ propertiesSetter = [](OpaqueProperties newProp,
+ const OpaqueProperties prop) {
+ *newProp.as<T *>() = *prop.as<const T *>();
+ };
+#if defined(__clang__)
+#if __has_warning("-Wdangling-assignment-gsl")
+#pragma clang diagnostic pop
+#endif
+#endif
+ 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 cdc1237ec8c5a..cc0fe924acf75 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -2504,6 +2504,13 @@ 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 55382a5bd3f8c..549830e06042f 100644
--- a/mlir/test/mlir-tblgen/op-attribute.td
+++ b/mlir/test/mlir-tblgen/op-attribute.td
@@ -165,6 +165,12 @@ 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.
@@ -279,6 +285,12 @@ 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 ee800a2952bac..a3dce9b31f8d2 100644
--- a/mlir/test/mlir-tblgen/op-decl-and-defs.td
+++ b/mlir/test/mlir-tblgen/op-decl-and-defs.td
@@ -119,6 +119,7 @@ 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();
@@ -231,6 +232,7 @@ 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
@@ -281,6 +283,8 @@ 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>]> {
@@ -293,6 +297,8 @@ 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,
@@ -329,6 +335,8 @@ 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 212d3189cf571..a4f7af6dbcf1c 100644
--- a/mlir/test/mlir-tblgen/op-result.td
+++ b/mlir/test/mlir-tblgen/op-result.td
@@ -57,7 +57,9 @@ 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>(attr.getValue()).getValue()});
+// 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()});
def OpE : NS_Op<"value_attr_as_result_type", [FirstAttrDerivedResultType]> {
let arguments = (ins I32:$x, F32Attr:$attr);
@@ -66,7 +68,10 @@ 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>(attr.getValue()).getType()});
+// 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()});
def OpF : NS_Op<"one_variadic_result_op", []> {
let results = (outs Variadic<I32>:$x);
@@ -118,6 +123,8 @@ 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 1970647a80115..b957c8ee9f8ab 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -411,6 +411,15 @@ 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;
@@ -661,24 +670,33 @@ 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();
+ void
+ 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();
+ void genInferredTypeCollectiveParamBuilder(CollectiveBuilderKind kind);
- // Generates the build() method that takes each operand/attribute as a
- // stand-alone parameter. The generated build() method uses first attribute's
+ // Generates the build() method that takesaggregate operands/attributes as
+ // parameters. The generated build() method uses first attribute's
// type as all result's types.
- void genUseAttrAsResultTypeBuilder();
+ void genUseAttrAsResultTypeCollectiveParamBuilder(CollectiveBuilderKind kind);
// Generates the build() method that takes all result types collectively as
// one parameter. Similarly for operands and attributes.
- void genCollectiveParamBuilder();
+ void genCollectiveParamBuilder(CollectiveBuilderKind kind);
// The kind of parameter to generate for result types in builders.
enum class TypeParamKind {
@@ -1363,8 +1381,6 @@ 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(
@@ -1728,6 +1744,9 @@ void OpEmitter::genPropertiesSupport() {
void OpEmitter::genPropertiesSupportForBytecode(
...
[truncated]
|
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/6650 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts commit 32f5437.
Investigations showed that the unit test utilities were calling erase(), causing a use-after-free. Fixed by rearranging checks in the test