diff --git a/mlir/docs/OpDefinitions.md b/mlir/docs/OpDefinitions.md --- a/mlir/docs/OpDefinitions.md +++ b/mlir/docs/OpDefinitions.md @@ -567,51 +567,104 @@ #### 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 different `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 --- a/mlir/include/mlir/IR/OpBase.td +++ b/mlir/include/mlir/IR/OpBase.td @@ -1804,6 +1804,13 @@ // 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 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 @@ // 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 { + 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 @@ // The signature of the builder is always // // ```c++ -// static void build(OpBuilder &builder, OperationState &state, +// static void build(::mlir::OpBuilder &builder, ::mlir::OperationState &state, // ...) { // ... // } // ``` // -// 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 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 : OpBuilderBase; + +// Deprecated version of OpBuilder that takes the builder signature as string. +class OpBuilder : OpBuilderBase<(ins), b> { + let params = p; } // A base decorator class that may optionally be added to OpVariables. @@ -2025,7 +2059,7 @@ // ValueRange operands, // ArrayRef attributes); // ``` - list builders = ?; + list 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 --- a/mlir/test/mlir-tblgen/op-decl.td +++ b/mlir/test/mlir-tblgen/op-decl.td @@ -33,7 +33,9 @@ AnyRegion:$someRegion, VariadicRegion:$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 @@ // 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_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 --- /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 { + let builders = [ + OpBuilderDAG<(outs)> + ]; +} +#endif + +#ifdef ERROR2 +// ERROR2: error: expected an argument with default value after other arguments with default values +def OpDefaultValueNotTrailing : Op { + 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 { + 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 --- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp +++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp @@ -1144,6 +1144,82 @@ 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 loc, + FmtContext &fctx) { + auto *defInit = dyn_cast(init->getOperator()); + if (!defInit || !defInit->getDef()->getName().equals("ins")) + PrintFatalError(loc, "expected 'ins' in builders"); + + // Inject builder and state arguments. + llvm::SmallVector 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(argInit)) { + type = strType->getValue(); + } else { + const Record *typeAndDefaultValue = cast(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 @@ if (listInit) { for (Init *init : listInit->getValues()) { Record *builderDef = cast(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 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()) {