[Mlir-commits] [mlir] 2a6db92 - [mlir][ods] Make OpBuilder and OperationState optional
Jacques Pienaar
llvmlistbot at llvm.org
Tue Sep 22 10:04:36 PDT 2020
Author: Jacques Pienaar
Date: 2020-09-22T10:04:21-07:00
New Revision: 2a6db92ca97da946307b559e63c6ac75caf4bbd6
URL: https://github.com/llvm/llvm-project/commit/2a6db92ca97da946307b559e63c6ac75caf4bbd6
DIFF: https://github.com/llvm/llvm-project/commit/2a6db92ca97da946307b559e63c6ac75caf4bbd6.diff
LOG: [mlir][ods] Make OpBuilder and OperationState optional
The OpBuilder is required to start with OpBuilder and OperationState, so remove
the need for the user to specify it. To make it simpler to update callers,
retain the legacy behavior for now and skip injecting OpBuilder/OperationState
when params start with OpBuilder.
Related to bug 47442.
Differential Revision: https://reviews.llvm.org/D88050
Added:
Modified:
mlir/docs/OpDefinitions.md
mlir/test/lib/Dialect/Test/TestOps.td
mlir/test/mlir-tblgen/op-decl.td
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
Removed:
################################################################################
diff --git a/mlir/docs/OpDefinitions.md b/mlir/docs/OpDefinitions.md
index 6e4a35035110..28af8fe2c33f 100644
--- a/mlir/docs/OpDefinitions.md
+++ b/mlir/docs/OpDefinitions.md
@@ -572,10 +572,13 @@ convenience build methods with `OpBuilder`.
`OpBuilder` is a class that takes the parameter list and the optional `build()`
method body. They are separated because we need to generate op declaration and
-definition into separate files. The parameter list should _include_ `Builder
-*builder, OperationState &state`. If the `body` is not provided, _only_ the
-builder declaration will be generated; this provides a way to define complicated
-builders entirely in C++ files.
+definition into separate files. The parameter list should not include `OpBuilder
+&builder, OperationState &state` as they will be inserted automatically and the
+placeholders `$_builder` and `$_state` used. For legacy/to be deprecated reason
+if the `OpBuilder` parameter starts with `OpBuilder` param, then the parameter
+is used. If the `body` is not provided, only the builder declaration will be
+generated; this provides a way to define complicated builders entirely in C++
+files.
For example, for the following op:
@@ -595,8 +598,8 @@ def MyOp : ... {
...
let builders = [
- OpBuilder<"OpBuilder &builder, OperationState &state, float val = 0.5f", [{
- state.addAttribute("attr", builder.getF32FloatAttr(val));
+ OpBuilder<"float val = 0.5f", [{
+ $_state.addAttribute("attr", $_builder.getF32FloatAttr(val));
}]>
];
}
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index bbff81884542..a2cf5b0d28e1 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -1062,14 +1062,14 @@ def MixedVResultOp3 : TEST_Op<"mixed_variadic_out3",
// result type. So need to provide a builder not requiring result types.
let builders = [
OpBuilder<
- "OpBuilder &builder, OperationState &state, IntegerAttr count",
+ "IntegerAttr count",
[{
- auto i32Type = builder.getIntegerType(32);
- state.addTypes(i32Type); // $output1
+ auto i32Type = $_builder.getIntegerType(32);
+ $_state.addTypes(i32Type); // $output1
SmallVector<Type, 4> types(count.getInt(), i32Type);
- state.addTypes(types); // $output2
- state.addTypes(types); // $output3
- state.addAttribute("count", count);
+ $_state.addTypes(types); // $output2
+ $_state.addTypes(types); // $output3
+ $_state.addAttribute("count", count);
}]>
];
}
diff --git a/mlir/test/mlir-tblgen/op-decl.td b/mlir/test/mlir-tblgen/op-decl.td
index 8390dea18ae9..0392264440f1 100644
--- a/mlir/test/mlir-tblgen/op-decl.td
+++ b/mlir/test/mlir-tblgen/op-decl.td
@@ -80,7 +80,7 @@ def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> {
// CHECK: uint32_t attr1();
// CHECK: ::mlir::FloatAttr attr2Attr()
// CHECK: ::llvm::Optional< ::llvm::APFloat > attr2();
-// CHECK: static void build(Value val);
+// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, Value val);
// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::Type r, ::llvm::ArrayRef<::mlir::Type> s, ::mlir::Value a, ::mlir::ValueRange b, ::mlir::IntegerAttr attr1, /*optional*/::mlir::FloatAttr attr2, unsigned someRegionsCount)
// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::Type r, ::llvm::ArrayRef<::mlir::Type> s, ::mlir::Value a, ::mlir::ValueRange b, uint32_t attr1, /*optional*/::mlir::FloatAttr attr2, unsigned someRegionsCount)
// CHECK: static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::llvm::ArrayRef<::mlir::Type> resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes, unsigned numRegions)
@@ -256,7 +256,7 @@ def NS_SkipDefaultBuildersOp : NS_Op<"skip_default_builders", []> {
// CHECK-LABEL: NS::SkipDefaultBuildersOp declarations
// CHECK: class SkipDefaultBuildersOp
// CHECK-NOT: static void build(::mlir::Builder
-// CHECK: static void build(Value
+// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, Value
// Check leading underscore in op name
// ---
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index ecadd20cd982..58a1b33e4acf 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -47,6 +47,7 @@ static cl::opt<std::string> opExcFilter(
static const char *const tblgenNamePrefix = "tblgen_";
static const char *const generatedArgName = "odsArg";
+static const char *const builder = "odsBuilder";
static const char *const builderOpState = "odsState";
// The logic to calculate the actual value range for a declared operand/result
@@ -1177,16 +1178,34 @@ void OpEmitter::genBuilder() {
if (listInit) {
for (Init *init : listInit->getValues()) {
Record *builderDef = cast<DefInit>(init)->getDef();
- StringRef params = builderDef->getValueAsString("params");
+ StringRef params = builderDef->getValueAsString("params").trim();
+ // TODO: Remove this and just generate the builder/state always.
+ bool skipParamGen = params.startswith("OpBuilder") ||
+ params.startswith("mlir::OpBuilder") ||
+ params.startswith("::mlir::OpBuilder");
StringRef body = builderDef->getValueAsString("body");
bool hasBody = !body.empty();
OpMethod::Property properties =
hasBody ? OpMethod::MP_Static : OpMethod::MP_StaticDeclaration;
+ std::string paramStr =
+ skipParamGen ? params.str()
+ : llvm::formatv("::mlir::OpBuilder &{0}, "
+ "::mlir::OperationState &{1}, {2}",
+ builder, builderOpState, params)
+ .str();
auto *method =
- opClass.addMethodAndPrune("void", "build", properties, params);
- if (hasBody)
- method->body() << body;
+ opClass.addMethodAndPrune("void", "build", properties, paramStr);
+ if (hasBody) {
+ if (skipParamGen) {
+ method->body() << body;
+ } else {
+ FmtContext fctx;
+ fctx.withBuilder(builder);
+ fctx.addSubst("_state", builderOpState);
+ method->body() << tgfmt(body, &fctx);
+ }
+ }
}
}
if (op.skipDefaultBuilders()) {
More information about the Mlir-commits
mailing list