[Mlir-commits] [mlir] 6ec3872 - [mlir] ODS: support TableGen dag objects to specify OpBuilder parameters
Alex Zinenko
llvmlistbot at llvm.org
Wed Oct 21 02:42:59 PDT 2020
Author: Alex Zinenko
Date: 2020-10-21T11:42:50+02:00
New Revision: 6ec3872845dbb4d98a9e21ba43428ba2c023209b
URL: https://github.com/llvm/llvm-project/commit/6ec3872845dbb4d98a9e21ba43428ba2c023209b
DIFF: https://github.com/llvm/llvm-project/commit/6ec3872845dbb4d98a9e21ba43428ba2c023209b.diff
LOG: [mlir] ODS: support TableGen dag objects to specify OpBuilder parameters
Historically, custom builder specification in OpBuilder has been accepting the
formal parameter list for the builder method as a raw string containing C++.
While this worked well to connect the signature and the body, this became
problematic when ODS needs to manipulate the parameter list, e.g. to inject
OpBuilder or to trim default values when generating the definition. This has
also become inconsistent with other method declarations, in particular in
interface definitions.
Introduce the possibility to define OpBuilder formal parameters using a
TableGen dag similarly to other methods. Additionally, introduce a mechanism to
declare parameters with default values using an additional class. This
mechanism can be reused in other methods. The string-based builder signature
declaration is deprecated and will be removed after a transition period.
Reviewed By: jpienaar
Differential Revision: https://reviews.llvm.org/D89470
Added:
mlir/test/mlir-tblgen/op-error.td
Modified:
mlir/docs/OpDefinitions.md
mlir/include/mlir/IR/OpBase.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 96c257cef08c..f44765204dc0 100644
--- a/mlir/docs/OpDefinitions.md
+++ b/mlir/docs/OpDefinitions.md
@@ -567,51 +567,104 @@ complete list.
#### Custom builder methods
However, if the above cases cannot satisfy all needs, you can define additional
-convenience build methods with `OpBuilder`.
+convenience build methods in the `builders` field as follows.
-`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 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.
+```tablegen
+def MyOp : Op<"my_op", []> {
+ let arguments = (ins F32Attr:$attr);
+
+ let builders = [
+ OpBuilderDAG<(ins "float":$val)>
+ ];
+}
+```
+
+The `builders` field is a list of custom builders that are added to the Op
+class. In this example, we provide a convenience builder that takes a floating
+point value instead of an attribute. The `ins` prefix is common to many function
+declarations in ODS, which use a TableGen [`dag`](#tablegen-syntax). What
+follows is a comma-separated list of types (quoted string) and names prefixed
+with the `$` sign. This will generate the declaration of a builder method that
+looks like:
+
+```c++
+class MyOp : /*...*/ {
+ /*...*/
+ static void build(::mlir::OpBuilder &builder, ::mlir::OperationState &state,
+ float val);
+};
+```
-For example, for the following op:
+Note that the method has two additional leading arguments. These arguments are
+useful to construct the operation. In particular, the method must populate
+`state` with attributes, operands, regions and result types of the operation to
+be constructed. `builder` can be used to construct any IR objects that belong to
+the Op, such as types or nested operations. Since the type and name are
+generated as is in the C++ code, they should be valid C++ constructs for a type
+(in the namespace of the Op) and an identifier (e.g., `class` is not a valid
+identifier).
+
+Implementations of the builder can be provided directly in ODS, using TableGen
+code block as follows.
```tablegen
def MyOp : Op<"my_op", []> {
let arguments = (ins F32Attr:$attr);
- let results = (outs);
+ let builders = [
+ OpBuilderDAG<(ins "float":$val), [{
+ $_state.addAttribute("attr", $_builder.getF32FloatAttr(val));
+ }]>
+ ];
}
```
-If we want to define a builder with a default value for the only attribute, we
-can add into `MyOp`:
+The equivalents of `builder` and `state` arguments are available as `$_builder`
+and `$_state` special variables. The named arguments listed in the `ins` part
+are available directly, e.g. `val`. The body of the builder will be generated by
+substituting special variables and should otherwise be valid C++. While there is
+no limitation on the code size, we encourage one to define only short builders
+inline in ODS and put definitions of longer builders in C++ files.
+
+Finally, if some arguments need a default value, they can be defined using
+`CArg` to wrap the type and this value as follows.
```tablegen
-def MyOp : ... {
- ...
+def MyOp : Op<"my_op", []> {
+ let arguments = (ins F32Attr:$attr);
let builders = [
- OpBuilder<"float val = 0.5f", [{
+ OpBuilderDAG<(ins CArg<"float", "0.5f">:$val), [{
$_state.addAttribute("attr", $_builder.getF32FloatAttr(val));
}]>
];
}
```
-The generated builder will look like:
+The generated code will use default value in the declaration, but not in the
+definition, as required by C++.
```c++
-static void build(OpBuilder &builder, OperationState &state, float val = 0.5f) {
+/* Header file. */
+class MyOp : /*...*/ {
+ /*...*/
+ static void build(::mlir::OpBuilder &builder, ::mlir::OperationState &state,
+ float val = 0.5f);
+};
+
+/* Source file. */
+MyOp::build(::mlir::OpBuilder &builder, ::mlir::OperationState &state,
+ float val) {
state.addAttribute("attr", builder.getF32FloatAttr(val));
}
```
+**Deprecated:** `OpBuilder` class allows one to specify the custom builder
+signature as a raw string, without separating parameters into
diff erent `dag`
+arguments. It also supports leading parameters of `OpBuilder &` and
+`OperationState &` types, which will be used instead of the autogenerated ones
+if present.
+
### Custom parser and printer methods
Functions to parse and print the operation's custom assembly form.
diff --git a/mlir/include/mlir/IR/OpBase.td b/mlir/include/mlir/IR/OpBase.td
index e09c18c0e1a1..82ef2dffca06 100644
--- a/mlir/include/mlir/IR/OpBase.td
+++ b/mlir/include/mlir/IR/OpBase.td
@@ -1804,6 +1804,13 @@ def NoRegionArguments : NativeOpTrait<"NoRegionArguments">;
// Marker used to identify the argument list for an op or interface method.
def ins;
+// This class represents a typed argument with optional default value for C
+// function signatures, e.g. builders or methods.
+class CArg<string ty, string value = ""> {
+ string type = ty;
+ string defaultValue = value;
+}
+
// OpInterfaceTrait corresponds to a specific 'OpInterface' class defined in
// C++. The purpose to wrap around C++ symbol string with this class is to make
// interfaces specified for ops in TableGen less alien and more integrated.
@@ -1923,6 +1930,15 @@ def region;
// Marker used to identify the successor list for an op.
def successor;
+// Base class for custom builders. This is a transient class that will go away
+// when the transition to the DAG form of builder declaration is complete.
+// Should not be used directly.
+class OpBuilderBase<dag dp, code b> {
+ string params = ?;
+ dag dagParams = dp;
+ code body = b;
+}
+
// Class for defining a custom builder.
//
// TableGen generates several generic builders for each op by default (see
@@ -1932,22 +1948,40 @@ def successor;
// The signature of the builder is always
//
// ```c++
-// static void build(OpBuilder &builder, OperationState &state,
+// static void build(::mlir::OpBuilder &builder, ::mlir::OperationState &state,
// <other-parameters>...) {
// <body>...
// }
// ```
//
-// To define a custom builder, the parameter list (*excluding* the `Builder
-// *builder, OperationState &state` part) and body should be passed in
-// as separate template arguments to this class. This is because we generate
-// op declaration and definition into separate files. If an empty string is
-// passed in for `body`, then *only* the builder declaration will be
-// generated; this provides a way to define complicated builders entirely
-// in C++.
-class OpBuilder<string p, code b = ""> {
- string params = p;
- code body = b;
+// To define a custom builder, the parameter list (*excluding* the
+// `OpBuilder &builder, OperationState &state` part) and body should be passed
+// in as separate template arguments to this class. The parameter list is a
+// TableGen DAG with `ins` operation with named arguments, which has either:
+// - string initializers ("Type":$name) to represent a typed parameter, or
+// - CArg-typed initializers (CArg<"Type", "default">:$name) to represent a
+// typed parameter that may have a default value.
+// The type string is used verbatim to produce code and, therefore, must be a
+// valid C++ type. It is used inside the C++ namespace of the parent Op's
+// dialect; explicit namespace qualification like `::mlir` may be necessary if
+// Ops are not placed inside the `mlir` namespace. The default value string is
+// used verbatim to produce code and must be a valid C++ initializer the given
+// type. For example, the following signature specification
+//
+// ```
+// OpBuilderDAG<(ins "int":$integerArg, CArg<"float", "3.0f">:$floatArg)>
+// ```
+//
+// has an integer parameter and a float parameter with a default value.
+//
+// If an empty string is passed in for `body`, then *only* the builder
+// declaration will be generated; this provides a way to define complicated
+// builders entirely in C++.
+class OpBuilderDAG<dag p, code b = ""> : OpBuilderBase<p, b>;
+
+// Deprecated version of OpBuilder that takes the builder signature as string.
+class OpBuilder<string p, code b = ""> : OpBuilderBase<(ins), b> {
+ let params = p;
}
// A base decorator class that may optionally be added to OpVariables.
@@ -2025,7 +2059,7 @@ class Op<Dialect dialect, string mnemonic, list<OpTrait> props = []> {
// ValueRange operands,
// ArrayRef<NamedAttribute> attributes);
// ```
- list<OpBuilder> builders = ?;
+ list<OpBuilderBase> builders = ?;
// Avoid generating default build functions. Custom builders must be
// provided.
diff --git a/mlir/test/mlir-tblgen/op-decl.td b/mlir/test/mlir-tblgen/op-decl.td
index 4ff77dc2c3f7..29438f1836a7 100644
--- a/mlir/test/mlir-tblgen/op-decl.td
+++ b/mlir/test/mlir-tblgen/op-decl.td
@@ -33,7 +33,9 @@ def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> {
AnyRegion:$someRegion,
VariadicRegion<AnyRegion>:$someRegions
);
- let builders = [OpBuilder<"Value val">];
+ let builders = [OpBuilderDAG<(ins "Value":$val)>,
+ OpBuilderDAG<(ins CArg<"int", "0">:$integer)>,
+ OpBuilder<"double deprecatedForm">];
let parser = [{ foo }];
let printer = [{ bar }];
let verifier = [{ baz }];
@@ -81,6 +83,8 @@ def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> {
// CHECK: ::mlir::FloatAttr attr2Attr()
// CHECK: ::llvm::Optional< ::llvm::APFloat > attr2();
// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, Value val);
+// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, int integer = 0);
+// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, double deprecatedForm);
// CHECK: static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::Type r, ::mlir::TypeRange 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, ::mlir::TypeRange 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, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes, unsigned numRegions)
@@ -250,7 +254,7 @@ def NS_JOp : NS_Op<"op_with_InferTypeOpInterface_interface", [DeclareOpInterface
def NS_SkipDefaultBuildersOp : NS_Op<"skip_default_builders", []> {
let skipDefaultBuilders = 1;
- let builders = [OpBuilder<"Value val">];
+ let builders = [OpBuilderDAG<(ins "Value":$val)>];
}
// CHECK-LABEL: NS::SkipDefaultBuildersOp declarations
diff --git a/mlir/test/mlir-tblgen/op-error.td b/mlir/test/mlir-tblgen/op-error.td
new file mode 100644
index 000000000000..b5fea66287a9
--- /dev/null
+++ b/mlir/test/mlir-tblgen/op-error.td
@@ -0,0 +1,36 @@
+// RUN: not mlir-tblgen -gen-op-decls -I %S/../../include -DERROR1 %s 2>&1 | FileCheck --check-prefix=ERROR1 %s
+// RUN: not mlir-tblgen -gen-op-decls -I %S/../../include -DERROR2 %s 2>&1 | FileCheck --check-prefix=ERROR2 %s
+// RUN: not mlir-tblgen -gen-op-decls -I %S/../../include -DERROR3 %s 2>&1 | FileCheck --check-prefix=ERROR3 %s
+
+include "mlir/IR/OpBase.td"
+
+def Test_Dialect : Dialect {
+ let name = "test_dialect";
+}
+
+#ifdef ERROR1
+// ERROR1: error: expected 'ins'
+def OpInsMissing : Op<Test_Dialect, "ins_missing"> {
+ let builders = [
+ OpBuilderDAG<(outs)>
+ ];
+}
+#endif
+
+#ifdef ERROR2
+// ERROR2: error: expected an argument with default value after other arguments with default values
+def OpDefaultValueNotTrailing : Op<Test_Dialect, "default_value"> {
+ let builders = [
+ OpBuilderDAG<(ins CArg<"int", "42">, "int")>
+ ];
+}
+#endif
+
+#ifdef ERROR3
+// ERROR3: error: expected an argument with default value after other arguments with default values
+def OpDefaultValueNotTrailing : Op<Test_Dialect, "default_value"> {
+ let builders = [
+ OpBuilderDAG<(ins CArg<"int", "42">, CArg<"int">)>
+ ];
+}
+#endif
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 3bcf02114555..f296b3284907 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -1144,6 +1144,82 @@ void OpEmitter::genUseAttrAsResultTypeBuilder() {
body << " }\n";
}
+/// Returns a signature of the builder as defined by a dag-typed initializer.
+/// Updates the context `fctx` to enable replacement of $_builder and $_state
+/// in the body. Reports errors at `loc`.
+static std::string builderSignatureFromDAG(const DagInit *init,
+ ArrayRef<llvm::SMLoc> loc,
+ FmtContext &fctx) {
+ auto *defInit = dyn_cast<DefInit>(init->getOperator());
+ if (!defInit || !defInit->getDef()->getName().equals("ins"))
+ PrintFatalError(loc, "expected 'ins' in builders");
+
+ // Inject builder and state arguments.
+ llvm::SmallVector<std::string, 8> arguments;
+ arguments.reserve(init->getNumArgs() + 2);
+ arguments.push_back(llvm::formatv("::mlir::OpBuilder &{0}", builder).str());
+ arguments.push_back(
+ llvm::formatv("::mlir::OperationState &{0}", builderOpState).str());
+
+ // Accept either a StringInit or a DefInit with two string values as dag
+ // arguments. The former corresponds to the type, the latter to the type and
+ // the default value. Similarly to C++, once an argument with a default value
+ // is detected, the following arguments must have default values as well.
+ bool seenDefaultValue = false;
+ for (unsigned i = 0, e = init->getNumArgs(); i < e; ++i) {
+ // If no name is provided, generate one.
+ StringInit *argName = init->getArgName(i);
+ std::string name =
+ argName ? argName->getValue().str() : "odsArg" + std::to_string(i);
+
+ Init *argInit = init->getArg(i);
+ StringRef type;
+ std::string defaultValue;
+ if (StringInit *strType = dyn_cast<StringInit>(argInit)) {
+ type = strType->getValue();
+ } else {
+ const Record *typeAndDefaultValue = cast<DefInit>(argInit)->getDef();
+ type = typeAndDefaultValue->getValueAsString("type");
+ StringRef defaultValueRef =
+ typeAndDefaultValue->getValueAsString("defaultValue");
+ if (!defaultValueRef.empty()) {
+ seenDefaultValue = true;
+ defaultValue = llvm::formatv(" = {0}", defaultValueRef).str();
+ }
+ }
+ if (seenDefaultValue && defaultValue.empty())
+ PrintFatalError(loc,
+ "expected an argument with default value after other "
+ "arguments with default values");
+ arguments.push_back(
+ llvm::formatv("{0} {1}{2}", type, name, defaultValue).str());
+ }
+
+ fctx.withBuilder(builder);
+ fctx.addSubst("_state", builderOpState);
+
+ return llvm::join(arguments, ", ");
+}
+
+// Returns a signature fo the builder as defined by a string initializer,
+// optionally injecting the builder and state arguments.
+// TODO: to be removed after the transition is complete.
+static std::string builderSignatureFromString(StringRef params,
+ FmtContext &fctx) {
+ bool skipParamGen = params.startswith("OpBuilder") ||
+ params.startswith("mlir::OpBuilder") ||
+ params.startswith("::mlir::OpBuilder");
+ if (skipParamGen)
+ return params.str();
+
+ fctx.withBuilder(builder);
+ fctx.addSubst("_state", builderOpState);
+ return std::string(llvm::formatv("::mlir::OpBuilder &{0}, "
+ "::mlir::OperationState &{1}{2}{3}",
+ builder, builderOpState,
+ params.empty() ? "" : ", ", params));
+}
+
void OpEmitter::genBuilder() {
// Handle custom builders if provided.
// TODO: Create wrapper class for OpBuilder to hide the native
@@ -1153,35 +1229,23 @@ void OpEmitter::genBuilder() {
if (listInit) {
for (Init *init : listInit->getValues()) {
Record *builderDef = cast<DefInit>(init)->getDef();
- 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");
+ llvm::Optional<StringRef> params =
+ builderDef->getValueAsOptionalString("params");
+ FmtContext fctx;
+ std::string paramStr =
+ params.hasValue() ? builderSignatureFromString(params->trim(), fctx)
+ : builderSignatureFromDAG(
+ builderDef->getValueAsDag("dagParams"),
+ op.getLoc(), fctx);
+
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}{3}",
- builder, builderOpState,
- params.empty() ? "" : ", ", params)
- .str();
auto *method =
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 (hasBody)
+ method->body() << tgfmt(body, &fctx);
}
}
if (op.skipDefaultBuilders()) {
More information about the Mlir-commits
mailing list