From d5524388ab7d1e2c3426cb53f3eec73b3c6a25ef Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Wed, 27 Mar 2019 14:09:19 -0700 Subject: [PATCH] [TableGen] Change names for Builder* and OperationState* parameters to avoid collision The `Builder*` parameter is unused in both generated build() methods so that we can leave it unnamed. Changed stand-alone parameter build() to take `_tblgen_state` instead of `result` to allow `result` to avoid having name collisions with op operand, attribute, or result. PiperOrigin-RevId: 240637700 --- mlir/include/mlir/IR/OpBase.td | 4 +-- mlir/test/mlir-tblgen/op-attribute.td | 4 +-- mlir/test/mlir-tblgen/op-decl.td | 4 +-- mlir/test/mlir-tblgen/op-operand.td | 10 ++++++++ mlir/test/mlir-tblgen/op-result.td | 26 +++++++++---------- mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp | 40 +++++++++++++++++------------ 6 files changed, 53 insertions(+), 35 deletions(-) diff --git a/mlir/include/mlir/IR/OpBase.td b/mlir/include/mlir/IR/OpBase.td index b921d91..7a441ef 100644 --- a/mlir/include/mlir/IR/OpBase.td +++ b/mlir/include/mlir/IR/OpBase.td @@ -622,7 +622,7 @@ class Op props = []> { // are generated, with the following signatures: // // ```c++ - // static void build(Builder* builder, OperationState* result, + // static void build(Builder *, OperationState *tblgen_state, // Type , Type , ..., // Value , Value , ..., // Attribute , Attribute , ...); @@ -630,7 +630,7 @@ class Op props = []> { // * where the attributes follow the same declaration order as in the op. // // ```c++ - // static void build(Builder* builder, OperationState* result, + // static void build(Builder *, OperationState *tblgen_state, // ArrayRef resultTypes, // ArrayRef operands, // ArrayRef attributes); diff --git a/mlir/test/mlir-tblgen/op-attribute.td b/mlir/test/mlir-tblgen/op-attribute.td index a3a71e6..6040aaa 100644 --- a/mlir/test/mlir-tblgen/op-attribute.td +++ b/mlir/test/mlir-tblgen/op-attribute.td @@ -9,7 +9,7 @@ def MixOperandsAndAttrs : Op<"mix_operands_and_attrs", []> { // CHECK-LABEL: MixOperandsAndAttrs definitions // CHECK-DAG: Value *MixOperandsAndAttrs::operand() // CHECK-DAG: Value *MixOperandsAndAttrs::otherArg() -// CHECK-DAG: void MixOperandsAndAttrs::build(Builder *builder, OperationState *result, FloatAttr attr, Value *operand, FloatAttr otherAttr, Value *otherArg) +// CHECK-DAG: void MixOperandsAndAttrs::build(Builder *, OperationState *tblgen_state, FloatAttr attr, Value *operand, FloatAttr otherAttr, Value *otherArg) // CHECK-DAG: APFloat MixOperandsAndAttrs::attr() // CHECK-DAG: APFloat MixOperandsAndAttrs::otherAttr() @@ -18,7 +18,7 @@ def OpWithArgs : Op<"op_with_args", []> { } // CHECK-LABEL: OpWithArgs definitions -// CHECK-DAG: void OpWithArgs::build(Builder *builder, OperationState *result, Value *x, FloatAttr attr, /*optional*/FloatAttr optAttr) +// CHECK-DAG: void OpWithArgs::build(Builder *, OperationState *tblgen_state, Value *x, FloatAttr attr, /*optional*/FloatAttr optAttr) // CHECK-DAG: APFloat OpWithArgs::attr() // CHECK-DAG: Optional< APFloat > OpWithArgs::optAttr() diff --git a/mlir/test/mlir-tblgen/op-decl.td b/mlir/test/mlir-tblgen/op-decl.td index 24dd2dc..a36eebc 100644 --- a/mlir/test/mlir-tblgen/op-decl.td +++ b/mlir/test/mlir-tblgen/op-decl.td @@ -38,8 +38,8 @@ def NS_AOp : Op<"a_op", [NoSideEffect]> { // CHECK: APInt attr1(); // CHECK: Optional< APFloat > attr2(); // CHECK: static void build(Value *val); -// CHECK: static void build(Builder *builder, OperationState *result, Type r, ArrayRef s, Value *a, ArrayRef b, IntegerAttr attr1, /*optional*/FloatAttr attr2); -// CHECK: static void build(Builder *builder, OperationState *result, ArrayRef resultTypes, ArrayRef args, ArrayRef attributes); +// CHECK: static void build(Builder *, OperationState *tblgen_state, Type r, ArrayRef s, Value *a, ArrayRef b, IntegerAttr attr1, /*optional*/FloatAttr attr2); +// CHECK: static void build(Builder *, OperationState *tblgen_state, ArrayRef resultTypes, ArrayRef operands, ArrayRef attributes); // CHECK: static bool parse(OpAsmParser *parser, OperationState *result); // CHECK: void print(OpAsmPrinter *p); // CHECK: bool verify(); diff --git a/mlir/test/mlir-tblgen/op-operand.td b/mlir/test/mlir-tblgen/op-operand.td index 7849be2..8f5b9f8 100644 --- a/mlir/test/mlir-tblgen/op-operand.td +++ b/mlir/test/mlir-tblgen/op-operand.td @@ -7,6 +7,16 @@ def OneOperandOp : Op<"one_operand_op", []> { } // CHECK-LABEL: OneOperandOp definitions + +// CHECK: void OneOperandOp::build +// CHECK-SAME: Value *input +// CHECK: tblgen_state->addOperands({input}); + +// CHECK: void OneOperandOp::build +// CHECK-SAME: ArrayRef operands +// CHECK: assert(operands.size() == 1u && "mismatched number of parameters"); +// CHECK: tblgen_state->addOperands(operands); + // CHECK: bool OneOperandOp::verify() { // CHECK: if (!((this->getOperation()->getOperand(0)->getType().isInteger(32)))) // CHECK-NEXT: return emitOpError("operand #0 must be 32-bit integer"); diff --git a/mlir/test/mlir-tblgen/op-result.td b/mlir/test/mlir-tblgen/op-result.td index d9e8295..d1e18d0 100644 --- a/mlir/test/mlir-tblgen/op-result.td +++ b/mlir/test/mlir-tblgen/op-result.td @@ -18,18 +18,18 @@ def SameTypeOp : Op<"same_type_op", [SameValueType]> { } // CHECK-LABEL: SameTypeOp definitions -// CHECK: void SameTypeOp::build(Builder *builder, OperationState *result, Type y, Value *x) -// CHECK: result->addTypes({y}); -// CHECK: void SameTypeOp::build(Builder *builder, OperationState *result, Value *x) -// CHECK: result->addTypes({x->getType()}); +// CHECK: void SameTypeOp::build(Builder *, OperationState *tblgen_state, Type y, Value *x) +// CHECK: tblgen_state->addTypes({y}); +// CHECK: void SameTypeOp::build(Builder *, OperationState *tblgen_state, Value *x) +// CHECK: tblgen_state->addTypes({x->getType()}); def ThreeResultOp : Op<"three_result_op", []> { let results = (outs I32:$x, /*unnamed*/I32, I32:$z); } // CHECK-LABEL: ThreeResultOp definitions -// CHECK: void ThreeResultOp::build(Builder *builder, OperationState *result, Type x, Type resultType1, Type z) -// CHECK: result->addTypes({x, resultType1, z}); +// CHECK: void ThreeResultOp::build(Builder *, OperationState *tblgen_state, Type x, Type resultType1, Type z) +// CHECK: tblgen_state->addTypes({x, resultType1, z}); def IntegerTypeAttr : TypeAttrBase<"IntegerType", "Integer type attribute">; def TypeAttrResultTypeOp : Op<"type_attr_as_result_type", [FirstAttrDerivedResultType]> { @@ -38,8 +38,8 @@ def TypeAttrResultTypeOp : Op<"type_attr_as_result_type", [FirstAttrDerivedResul } // CHECK-LABEL: TypeAttrResultTypeOp definitions -// CHECK: void TypeAttrResultTypeOp::build(Builder *builder, OperationState *result, Value *x, TypeAttr attr, FloatAttr f32) -// CHECK: result->addTypes({attr.getValue()}); +// CHECK: void TypeAttrResultTypeOp::build(Builder *, OperationState *tblgen_state, Value *x, TypeAttr attr, FloatAttr f32) +// CHECK: tblgen_state->addTypes({attr.getValue()}); def ValueAttrResultTypeOp : Op<"value_attr_as_result_type", [FirstAttrDerivedResultType]> { let arguments = (ins I32:$x, F32Attr:$attr); @@ -47,14 +47,14 @@ def ValueAttrResultTypeOp : Op<"value_attr_as_result_type", [FirstAttrDerivedRes } // CHECK-LABEL: ValueAttrResultTypeOp definitions -// CHECK: void ValueAttrResultTypeOp::build(Builder *builder, OperationState *result, Value *x, FloatAttr attr) -// CHECK: result->addTypes({attr.getType()}); +// CHECK: void ValueAttrResultTypeOp::build(Builder *, OperationState *tblgen_state, Value *x, FloatAttr attr) +// CHECK: tblgen_state->addTypes({attr.getType()}); def VariadicResultOp : Op<"variadic_op", []> { let results = (outs I32:$x, Variadic:$y); } // CHECK-LABEL: VariadicResultOp definitions -// CHECK: void VariadicResultOp::build(Builder *builder, OperationState *result, Type x, ArrayRef y) -// CHECK: result->addTypes({x}); -// CHECK: result->addTypes(y); +// CHECK: void VariadicResultOp::build(Builder *, OperationState *tblgen_state, Type x, ArrayRef y) +// CHECK: tblgen_state->addTypes({x}); +// CHECK: tblgen_state->addTypes(y); diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp index 198aa25..66f5b03 100644 --- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp +++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp @@ -36,6 +36,7 @@ using namespace mlir; using mlir::tblgen::Operator; +static const char *const builderOpState = "tblgen_state"; static const char *const generatedArgName = "_arg"; static const char *const opCommentHeader = R"( @@ -515,7 +516,8 @@ void OpEmitter::genStandaloneParamBuilder(bool useOperandType, llvm::SmallVector resultNames; resultNames.reserve(numResults); - std::string paramList = "Builder *builder, OperationState *result"; + std::string paramList = "Builder *, OperationState *"; + paramList.append(builderOpState); // Emit parameters for all return types if (!useOperandType && !useAttrType) { @@ -571,7 +573,8 @@ void OpEmitter::genStandaloneParamBuilder(bool useOperandType, numResults - static_cast(hasVariadicResult); if (numNonVariadicResults > 0) { - method.body() << " result->addTypes({" << resultNames.front(); + method.body() << " " << builderOpState << "->addTypes({" + << resultNames.front(); for (int i = 1; i < numNonVariadicResults; ++i) { method.body() << ", " << resultNames[i]; } @@ -579,7 +582,8 @@ void OpEmitter::genStandaloneParamBuilder(bool useOperandType, } if (hasVariadicResult) { - method.body() << " result->addTypes(" << resultNames.back() << ");\n"; + method.body() << " " << builderOpState << "->addTypes(" + << resultNames.back() << ");\n"; } } else { std::string resultType; @@ -593,7 +597,7 @@ void OpEmitter::genStandaloneParamBuilder(bool useOperandType, } else { resultType = formatv("{0}->getType()", getArgumentName(op, 0)).str(); } - method.body() << " result->addTypes({" << resultType; + method.body() << " " << builderOpState << "->addTypes({" << resultType; for (unsigned i = 1; i != numResults; ++i) method.body() << ", " << resultType; method.body() << "});\n\n"; @@ -605,14 +609,15 @@ void OpEmitter::genStandaloneParamBuilder(bool useOperandType, int numNonVariadicOperands = numOperands - static_cast(hasVariadicOperand); if (numNonVariadicOperands > 0) { - method.body() << " result->addOperands({" << getArgumentName(op, 0); + method.body() << " " << builderOpState << "->addOperands({" + << getArgumentName(op, 0); for (int i = 1; i < numNonVariadicOperands; ++i) { method.body() << ", " << getArgumentName(op, i); } method.body() << "});\n"; } if (hasVariadicOperand) { - method.body() << " result->addOperands(" + method.body() << " " << builderOpState << "->addOperands(" << getArgumentName(op, numOperands - 1) << ");\n"; } @@ -623,8 +628,9 @@ void OpEmitter::genStandaloneParamBuilder(bool useOperandType, if (emitNotNullCheck) { method.body() << formatv(" if ({0}) ", namedAttr.name) << "{\n"; } - method.body() << formatv(" result->addAttribute(\"{0}\", {1});\n", - namedAttr.getName(), namedAttr.name); + method.body() << formatv(" {0}->addAttribute(\"{1}\", {2});\n", + builderOpState, namedAttr.getName(), + namedAttr.name); if (emitNotNullCheck) { method.body() << " }\n"; } @@ -681,9 +687,10 @@ void OpEmitter::genBuilder() { // 2. Aggregated parameters // Signature - const char *const params = - "Builder *builder, OperationState *result, ArrayRef resultTypes, " - "ArrayRef args, ArrayRef attributes"; + std::string params = + std::string("Builder *, OperationState *") + builderOpState + + ", ArrayRef resultTypes, ArrayRef operands, " + "ArrayRef attributes"; auto &method = opClass.newMethod("void", "build", params, OpMethod::MP_Static); @@ -692,14 +699,14 @@ void OpEmitter::genBuilder() { << (hasVariadicResult ? " >= " : " == ") << numNonVariadicResults << "u && \"mismatched number of return types\");\n" - << " result->addTypes(resultTypes);\n"; + << " " << builderOpState << "->addTypes(resultTypes);\n"; // Operands - method.body() << " assert(args.size()" + method.body() << " assert(operands.size()" << (hasVariadicOperand ? " >= " : " == ") << numNonVariadicOperands << "u && \"mismatched number of parameters\");\n" - << " result->addOperands(args);\n\n"; + << " " << builderOpState << "->addOperands(operands);\n\n"; // Attributes if (op.getNumAttributes() > 0) { @@ -708,8 +715,9 @@ void OpEmitter::genBuilder() { } else { method.body() << " assert(attributes.size() >= " << op.getNumAttributes() << "u && \"not enough attributes\");\n" - << " for (const auto& pair : attributes)\n" - << " result->addAttribute(pair.first, pair.second);\n"; + << " for (const auto& pair : attributes)\n" + << " " << builderOpState + << "->addAttribute(pair.first, pair.second);\n"; } // 3. Deduced result types -- 2.7.4