Skip to content

Commit 1b2c23a

Browse files
krzysz00krzysz00
authored
Re-land "[mlir][ODS] Add a generated builder that takes the Properties struct" (#130117) (#130373)
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
1 parent 262a775 commit 1b2c23a

File tree

10 files changed

+241
-49
lines changed

10 files changed

+241
-49
lines changed

‎mlir/docs/DeclarativeRewrites.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,9 +237,9 @@ In the above, we are using `BOp`'s result for building `COp`.
237237

238238
Given that `COp` was specified with table-driven op definition, there will be
239239
several `build()` methods generated for it. One of them has aggregated
240-
parameters for result types, operands, and attributes in the signature: `void
240+
parameters for result types, operands, and properties in the signature: `void
241241
COp::build(..., ArrayRef<Type> resultTypes, Array<Value> operands,
242-
ArrayRef<NamedAttribute> attr)`. The pattern in the above calls this `build()`
242+
const COp::Properties& properties)`. The pattern in the above calls this `build()`
243243
method for constructing the `COp`.
244244

245245
In general, arguments in the result pattern will be passed directly to the

‎mlir/docs/DefiningDialects/Operations.md‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,18 @@ def MyOp : ... {
465465
The following builders are generated:
466466

467467
```c++
468+
// All result-types/operands/properties/discardable attributes have one
469+
// aggregate parameter. `Properties` is the properties structure of
470+
// `MyOp`.
471+
static void build(OpBuilder &odsBuilder, OperationState &odsState,
472+
TypeRange resultTypes,
473+
ValueRange operands,
474+
Properties properties,
475+
ArrayRef<NamedAttribute> discardableAttributes = {});
476+
468477
// All result-types/operands/attributes have one aggregate parameter.
478+
// Inherent properties and discardable attributes are mixed together in the
479+
// `attributes` dictionary.
469480
static void build(OpBuilder &odsBuilder, OperationState &odsState,
470481
TypeRange resultTypes,
471482
ValueRange operands,
@@ -498,20 +509,28 @@ static void build(OpBuilder &odsBuilder, OperationState &odsState,
498509

499510
// All operands/attributes have aggregate parameters.
500511
// Generated if return type can be inferred.
512+
static void build(OpBuilder &odsBuilder, OperationState &odsState,
513+
ValueRange operands,
514+
Properties properties,
515+
ArrayRef<NamedAttribute> discardableAttributes);
516+
517+
// All operands/attributes have aggregate parameters.
518+
// Generated if return type can be inferred. Uses the legacy merged attribute
519+
// dictionary.
501520
static void build(OpBuilder &odsBuilder, OperationState &odsState,
502521
ValueRange operands, ArrayRef<NamedAttribute> attributes);
503522

504523
// (And manually specified builders depending on the specific op.)
505524
```
506525
507-
The first form provides basic uniformity so that we can create ops using the
508-
same form regardless of the exact op. This is particularly useful for
526+
The first two forms provide basic uniformity so that we can create ops using
527+
the same form regardless of the exact op. This is particularly useful for
509528
implementing declarative pattern rewrites.
510529
511-
The second and third forms are good for use in manually written code, given that
530+
The third and fourth forms are good for use in manually written code, given that
512531
they provide better guarantee via signatures.
513532
514-
The third form will be generated if any of the op's attribute has different
533+
The fourth form will be generated if any of the op's attribute has different
515534
`Attr.returnType` from `Attr.storageType` and we know how to build an attribute
516535
from an unwrapped value (i.e., `Attr.constBuilderCall` is defined.)
517536
Additionally, for the third form, if an attribute appearing later in the

‎mlir/include/mlir/IR/OpDefinition.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,10 @@ void ensureRegionTerminator(
7474

7575
/// Structure used by default as a "marker" when no "Properties" are set on an
7676
/// Operation.
77-
struct EmptyProperties {};
77+
struct EmptyProperties {
78+
bool operator==(const EmptyProperties &) const { return true; }
79+
bool operator!=(const EmptyProperties &) const { return false; }
80+
};
7881

7982
/// Traits to detect whether an Operation defined a `Properties` type, otherwise
8083
/// it'll default to `EmptyProperties`.

‎mlir/include/mlir/IR/OperationSupport.h‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,6 +1029,36 @@ struct OperationState {
10291029
setProperties(Operation *op,
10301030
function_ref<InFlightDiagnostic()> emitError) const;
10311031

1032+
// Make `newProperties` the source of the properties that will be copied into
1033+
// the operation. The memory referenced by `newProperties` must remain live
1034+
// until after the `Operation` is created, at which time it may be
1035+
// deallocated. Calls to `getOrAddProperties<>() will return references to
1036+
// this memory.
1037+
template <typename T>
1038+
void useProperties(T &newProperties) {
1039+
assert(!properties &&
1040+
"Can't provide a properties struct when one has been allocated");
1041+
properties = &newProperties;
1042+
#if defined(__clang__)
1043+
#if __has_warning("-Wdangling-assignment-gsl")
1044+
#pragma clang diagnostic push
1045+
// https://github.com/llvm/llvm-project/issues/126600
1046+
#pragma clang diagnostic ignored "-Wdangling-assignment-gsl"
1047+
#endif
1048+
#endif
1049+
propertiesDeleter = [](OpaqueProperties) {};
1050+
propertiesSetter = [](OpaqueProperties newProp,
1051+
const OpaqueProperties prop) {
1052+
*newProp.as<T *>() = *prop.as<const T *>();
1053+
};
1054+
#if defined(__clang__)
1055+
#if __has_warning("-Wdangling-assignment-gsl")
1056+
#pragma clang diagnostic pop
1057+
#endif
1058+
#endif
1059+
propertiesId = TypeID::get<T>();
1060+
}
1061+
10321062
void addOperands(ValueRange newOperands);
10331063

10341064
void addTypes(ArrayRef<Type> newTypes) {

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2504,6 +2504,13 @@ def TableGenBuildOp6 : TEST_Op<"tblgen_build_6", [AttrSizedOperandSegments]> {
25042504
let results = (outs F32:$result);
25052505
}
25062506

2507+
// An inherent attribute. Test collective builders, both those that take properties as
2508+
// properties structs and those that take an attribute dictionary.
2509+
def TableGenBuildOp7 : TEST_Op<"tblgen_build_7", []> {
2510+
let arguments = (ins BoolAttr:$attr0);
2511+
let results = (outs);
2512+
}
2513+
25072514
//===----------------------------------------------------------------------===//
25082515
// Test BufferPlacement
25092516
//===----------------------------------------------------------------------===//

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,12 @@ def AOp : NS_Op<"a_op", []> {
165165
// DEF: ::llvm::ArrayRef<::mlir::NamedAttribute> attributes
166166
// DEF: odsState.addAttributes(attributes);
167167

168+
// DEF: void AOp::build(
169+
// DEF-SAME: const Properties &properties,
170+
// DEF-SAME: ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes
171+
// DEF: odsState.useProperties(const_cast<Properties&>(properties));
172+
// DEF: odsState.addAttributes(discardableAttributes);
173+
168174
// DEF: void AOp::populateDefaultProperties
169175

170176
// Test the above but with prefix.
@@ -279,6 +285,12 @@ def AgetOp : Op<Test2_Dialect, "a_get_op", []> {
279285
// DEF: ::llvm::ArrayRef<::mlir::NamedAttribute> attributes
280286
// DEF: odsState.addAttributes(attributes);
281287

288+
// DEF: void AgetOp::build(
289+
// DEF-SAME: const Properties &properties
290+
// DEF-SAME: ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes
291+
// DEF: odsState.useProperties(const_cast<Properties&>(properties));
292+
// DEF: odsState.addAttributes(discardableAttributes);
293+
282294
// Test the above but using properties.
283295
def ApropOp : NS_Op<"a_prop_op", []> {
284296
let arguments = (ins

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> {
119119
// 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)
120120
// 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);
121121
// CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes, unsigned numRegions)
122+
// 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)
122123
// CHECK: static ::mlir::ParseResult parse(::mlir::OpAsmParser &parser, ::mlir::OperationState &result);
123124
// CHECK: void print(::mlir::OpAsmPrinter &p);
124125
// CHECK: ::llvm::LogicalResult verifyInvariants();
@@ -231,6 +232,7 @@ def NS_HCollectiveParamsOp : NS_Op<"op_collective_params", []> {
231232
// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::Type b, ::mlir::Value a);
232233
// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::Value a);
233234
// CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {})
235+
// CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {})
234236

235237
// Check suppression of "separate arg, separate result" build method for an op
236238
// 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
281283
// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::Value a, ::mlir::Value b);
282284
// CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
283285
// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
286+
// CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
287+
// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
284288

285289
// Check default value of `attributes` for the `genInferredTypeCollectiveParamBuilder` builder
286290
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
293297
// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::Value a, ::mlir::Value b);
294298
// CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
295299
// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
300+
// CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
301+
// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
296302

297303
// Test usage of TraitList getting flattened during emission.
298304
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
329335
// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::Value a, ::mlir::Value b, uint32_t attr1);
330336
// CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
331337
// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes = {});
338+
// CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
339+
// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes = {});
332340

333341
def NS_MOp : NS_Op<"op_with_single_result_and_fold_adaptor_fold", []> {
334342
let results = (outs AnyType:$res);

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,9 @@ def OpD : NS_Op<"type_attr_as_result_type", [FirstAttrDerivedResultType]> {
5757

5858
// CHECK-LABEL: OpD definitions
5959
// CHECK: void OpD::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes)
60-
// CHECK: odsState.addTypes({::llvm::cast<::mlir::TypeAttr>(attr.getValue()).getValue()});
60+
// CHECK: odsState.addTypes({::llvm::cast<::mlir::TypeAttr>(typeAttr).getValue()});
61+
// CHECK: void OpD::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes)
62+
// CHECK: odsState.addTypes({::llvm::cast<::mlir::TypeAttr>(typeAttr).getValue()});
6163

6264
def OpE : NS_Op<"value_attr_as_result_type", [FirstAttrDerivedResultType]> {
6365
let arguments = (ins I32:$x, F32Attr:$attr);
@@ -66,7 +68,10 @@ def OpE : NS_Op<"value_attr_as_result_type", [FirstAttrDerivedResultType]> {
6668

6769
// CHECK-LABEL: OpE definitions
6870
// CHECK: void OpE::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes)
69-
// CHECK: odsState.addTypes({::llvm::cast<::mlir::TypedAttr>(attr.getValue()).getType()});
71+
// CHECK: odsState.addTypes({::llvm::cast<::mlir::TypedAttr>(typeAttr).getType()});
72+
// CHECK: void OpE::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes)
73+
// CHECK: ::mlir::Attribute typeAttr = properties.getAttr();
74+
// CHECK: odsState.addTypes({::llvm::cast<::mlir::TypedAttr>(typeAttr).getType()});
7075

7176
def OpF : NS_Op<"one_variadic_result_op", []> {
7277
let results = (outs Variadic<I32>:$x);
@@ -118,6 +123,8 @@ def OpK : NS_Op<"only_input_is_variadic_with_same_value_type_op", [SameOperandsA
118123

119124
// CHECK-LABEL: OpK::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes)
120125
// CHECK: odsState.addTypes({operands[0].getType()});
126+
// CHECK-LABEL: OpK::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::ValueRange operands, const Properties &properties, ::llvm::ArrayRef<::mlir::NamedAttribute> discardableAttributes)
127+
// CHECK: odsState.addTypes({operands[0].getType()});
121128

122129
// Test with inferred shapes and interleaved with operands/attributes.
123130
//

0 commit comments

Comments
 (0)