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,82 @@ return std::string(llvm::formatv("{0}Adaptor", getCppClassName())); } +void Operator::assertInvariants() const { + // Check that the name of arguments/results/regions/successors don't overlap. + + // Check operands amongst themselves. + llvm::SmallDenseSet operandNames; + for (int i : llvm::seq(0, getNumOperands())) { + StringRef name = getOperand(i).name; + if (name.empty()) + continue; + if (!operandNames.insert(name).second) + PrintFatalError(getLoc(), + "op has two operands with the same name: '" + name + "'"); + } + + // Check results amongst themselves and against operands. + llvm::SmallDenseSet resultNames; + for (int i : llvm::seq(0, getNumResults())) { + StringRef name = getResult(i).name; + if (name.empty()) + continue; + if (!resultNames.insert(name).second) + PrintFatalError(getLoc(), + "op has two results with the same name: '" + name + "'"); + if (operandNames.contains(name)) + PrintFatalError( + getLoc(), "op is using the same name for an operand and a result: '" + + name + "'"); + } + + // Check regions amongst themselves and against operands and results. + llvm::SmallDenseSet regionNames; + for (int i : llvm::seq(0, getNumRegions())) { + StringRef name = getRegion(i).name; + if (name.empty()) + continue; + if (!regionNames.insert(name).second) + PrintFatalError(getLoc(), + "op has two regions with the same name: '" + name + "'"); + if (operandNames.contains(name)) + PrintFatalError( + getLoc(), "op is using the same name for an operand and a region: '" + + name + "'"); + if (resultNames.contains(name)) + PrintFatalError(getLoc(), + "op is using the same name for a result and a region: '" + + name + "'"); + } + + // Check successors amongst themselves and against operands, results, and + // regions. + llvm::SmallDenseSet successorNames; + for (int i : llvm::seq(0, getNumSuccessors())) { + StringRef name = getSuccessor(i).name; + if (name.empty()) + continue; + if (!successorNames.insert(name).second) + PrintFatalError(getLoc(), "op has two successors with the same name: '" + + name + "'"); + if (operandNames.contains(name)) + PrintFatalError( + getLoc(), + "op is using the same name for an operand and a successor: '" + name + + "'"); + if (resultNames.contains(name)) + PrintFatalError( + getLoc(), + "op is using the same name for a result and a successor: '" + name + + "'"); + if (regionNames.contains(name)) + PrintFatalError( + getLoc(), + "op is using the same name for a region and a successor: '" + name + + "'"); + } +} + 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" @@ -50,3 +56,51 @@ let results = (outs AnyTensor:$tensor, AnyTensor:$tensor); } #endif + +#ifdef ERROR6 +// ERROR6: error: op is using the same name for an operand and a result: 'tensor' +def OpWithDuplicatedArgResultNames : Op { + let arguments = (ins AnyTensor:$tensor); + let results = (outs AnyTensor:$tensor); +} +#endif + +#ifdef ERROR7 +// ERROR7: error: op is using the same name for an operand and a region: 'tensor' +def OpWithDuplicatedArgResultNames : Op { + let arguments = (ins AnyTensor:$tensor); + let regions = (region AnyRegion:$tensor); +} +#endif + +#ifdef ERROR8 +// ERROR8: error: op is using the same name for a result and a region: 'tensor' +def OpWithDuplicatedArgResultNames : Op { + let results = (outs AnyTensor:$tensor); + let regions = (region AnyRegion:$tensor); +} +#endif + +#ifdef ERROR9 +// ERROR9: error: op is using the same name for an operand and a successor: 'target' +def OpWithDuplicatedArgResultNames : Op { + let successors = (successor AnySuccessor:$target); + let arguments = (ins AnyTensor:$target); +} +#endif + +#ifdef ERROR10 +// ERROR10: error: op is using the same name for a result and a successor: 'target' +def OpWithDuplicatedArgResultNames : Op { + let successors = (successor AnySuccessor:$target); + let results = (outs AnyTensor:$target); +} +#endif + +#ifdef ERROR11 +// ERROR11: error: op is using the same name for a region and a successor: '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()