diff --git a/mlir/include/mlir/TableGen/Operator.h b/mlir/include/mlir/TableGen/Operator.h --- a/mlir/include/mlir/TableGen/Operator.h +++ b/mlir/include/mlir/TableGen/Operator.h @@ -64,6 +64,10 @@ // Returns the name of op's adaptor C++ class. std::string getAdaptorName() const; + // Check invariants (like no duplicated or conflicted names) and abort the + // process if any invariant is broken. + void assertInvariants() const; + /// A class used to represent the decorators of an operator variable, i.e. /// argument or result. struct VariableDecorator { diff --git a/mlir/lib/TableGen/Operator.cpp b/mlir/lib/TableGen/Operator.cpp --- a/mlir/lib/TableGen/Operator.cpp +++ b/mlir/lib/TableGen/Operator.cpp @@ -53,6 +53,7 @@ cppNamespace = def.getValueAsString("cppNamespace"); populateOpStructure(); + assertInvariants(); } std::string Operator::getOperationName() const { @@ -67,6 +68,41 @@ return std::string(llvm::formatv("{0}Adaptor", getCppClassName())); } +void Operator::assertInvariants() const { + // Check that the name of arguments/results/regions/successors don't overlap. + DenseMap existingNames; + auto checkName = [&](StringRef name, StringRef entity) { + if (name.empty()) + return; + auto insertion = existingNames.insert({name, entity}); + if (insertion.second) + return; + if (entity == insertion.first->second) + PrintFatalError(getLoc(), "op has a conflict with two " + entity + + " having the same name '" + name + "'"); + PrintFatalError(getLoc(), "op has a conflict with " + + insertion.first->second + " and " + entity + + " both having an entry with the name '" + + name + "'"); + }; + // Check operands amongst themselves. + for (int i : llvm::seq(0, getNumOperands())) + checkName(getOperand(i).name, "operands"); + + // Check results amongst themselves and against operands. + for (int i : llvm::seq(0, getNumResults())) + checkName(getResult(i).name, "results"); + + // Check regions amongst themselves and against operands and results. + for (int i : llvm::seq(0, getNumRegions())) + checkName(getRegion(i).name, "regions"); + + // Check successors amongst themselves and against operands, results, and + // regions. + for (int i : llvm::seq(0, getNumSuccessors())) + checkName(getSuccessor(i).name, "successors"); +} + StringRef Operator::getDialectName() const { return dialect.getName(); } StringRef Operator::getCppClassName() const { return cppClassName; } diff --git a/mlir/test/mlir-tblgen/op-error.td b/mlir/test/mlir-tblgen/op-error.td --- a/mlir/test/mlir-tblgen/op-error.td +++ b/mlir/test/mlir-tblgen/op-error.td @@ -3,6 +3,12 @@ // RUN: not mlir-tblgen -gen-op-decls -I %S/../../include -DERROR3 %s 2>&1 | FileCheck --check-prefix=ERROR3 %s // RUN: not mlir-tblgen -gen-op-decls -I %S/../../include -DERROR4 %s 2>&1 | FileCheck --check-prefix=ERROR4 %s // RUN: not mlir-tblgen -gen-op-decls -I %S/../../include -DERROR5 %s 2>&1 | FileCheck --check-prefix=ERROR5 %s +// RUN: not mlir-tblgen -gen-op-decls -I %S/../../include -DERROR6 %s 2>&1 | FileCheck --check-prefix=ERROR6 %s +// RUN: not mlir-tblgen -gen-op-decls -I %S/../../include -DERROR7 %s 2>&1 | FileCheck --check-prefix=ERROR7 %s +// RUN: not mlir-tblgen -gen-op-decls -I %S/../../include -DERROR8 %s 2>&1 | FileCheck --check-prefix=ERROR8 %s +// RUN: not mlir-tblgen -gen-op-decls -I %S/../../include -DERROR9 %s 2>&1 | FileCheck --check-prefix=ERROR9 %s +// RUN: not mlir-tblgen -gen-op-decls -I %S/../../include -DERROR10 %s 2>&1 | FileCheck --check-prefix=ERROR10 %s +// RUN: not mlir-tblgen -gen-op-decls -I %S/../../include -DERROR11 %s 2>&1 | FileCheck --check-prefix=ERROR11 %s include "mlir/IR/OpBase.td" @@ -38,15 +44,63 @@ #endif #ifdef ERROR4 -// ERROR4: error: op has two operands with the same name: 'tensor' +// ERROR4: error: op has a conflict with two operands having the same name 'tensor' def OpWithDuplicatedArgNames : Op { let arguments = (ins AnyTensor:$tensor, AnyTensor:$tensor); } #endif #ifdef ERROR5 -// ERROR5: error: op has two results with the same name: 'tensor' +// ERROR5: error: op has a conflict with two results having the same name 'tensor' def OpWithDuplicatedResultNames : Op { let results = (outs AnyTensor:$tensor, AnyTensor:$tensor); } #endif + +#ifdef ERROR6 +// ERROR6: error: op has a conflict with operands and results both having an entry with the name 'tensor' +def OpWithDuplicatedArgResultNames : Op { + let arguments = (ins AnyTensor:$tensor); + let results = (outs AnyTensor:$tensor); +} +#endif + +#ifdef ERROR7 +// ERROR7: error: op has a conflict with operands and regions both having an entry with the name 'tensor' +def OpWithDuplicatedArgResultNames : Op { + let arguments = (ins AnyTensor:$tensor); + let regions = (region AnyRegion:$tensor); +} +#endif + +#ifdef ERROR8 +// ERROR8: error: op has a conflict with results and regions both having an entry with the name 'tensor' +def OpWithDuplicatedArgResultNames : Op { + let results = (outs AnyTensor:$tensor); + let regions = (region AnyRegion:$tensor); +} +#endif + +#ifdef ERROR9 +// ERROR9: error: op has a conflict with operands and successors both having an entry with the name 'target' +def OpWithDuplicatedArgResultNames : Op { + let successors = (successor AnySuccessor:$target); + let arguments = (ins AnyTensor:$target); +} +#endif + +#ifdef ERROR10 +// ERROR10: error: op has a conflict with results and successors both having an entry with the name 'target' +def OpWithDuplicatedArgResultNames : Op { + let successors = (successor AnySuccessor:$target); + let results = (outs AnyTensor:$target); +} +#endif + +#ifdef ERROR11 +// ERROR11: error: op has a conflict with regions and successors both having an entry with the name 'target' +def OpWithDuplicatedArgResultNames : Op { + let successors = (successor AnySuccessor:$target); + let regions = (region AnyRegion:$target); +} +#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 @@ -850,16 +850,10 @@ // Then we emit nicer named getter methods by redirecting to the "sink" getter // method. - // Keep track of the operand names to find duplicates. - SmallDenseSet operandNames; for (int i = 0; i != numOperands; ++i) { const auto &operand = op.getOperand(i); if (operand.name.empty()) continue; - if (!operandNames.insert(operand.name).second) - PrintFatalError(op.getLoc(), "op has two operands with the same name: '" + - operand.name + "'"); - if (operand.isOptional()) { m = opClass.addMethodAndPrune("::mlir::Value", operand.name); m->body() @@ -992,15 +986,10 @@ m->body() << formatv(valueRangeReturnCode, "getOperation()->result_begin()", "getODSResultIndexAndLength(index)"); - SmallDenseSet resultNames; for (int i = 0; i != numResults; ++i) { const auto &result = op.getResult(i); if (result.name.empty()) continue; - if (!resultNames.insert(result.name).second) - PrintFatalError(op.getLoc(), "op has two results with the same name: '" + - result.name + "'"); - if (result.isOptional()) { m = opClass.addMethodAndPrune("::mlir::Value", result.name); m->body()