[Mlir-commits] [mlir] 7fb2394 - Add sanity check in MLIR ODS to catch case where an arguments/results/regions/successors names overlap
Mehdi Amini
llvmlistbot at llvm.org
Sun Sep 12 23:21:36 PDT 2021
Author: Mehdi Amini
Date: 2021-09-13T06:21:25Z
New Revision: 7fb2394a4f362aff4282af8486b7352e720c32ab
URL: https://github.com/llvm/llvm-project/commit/7fb2394a4f362aff4282af8486b7352e720c32ab
DIFF: https://github.com/llvm/llvm-project/commit/7fb2394a4f362aff4282af8486b7352e720c32ab.diff
LOG: Add sanity check in MLIR ODS to catch case where an arguments/results/regions/successors names overlap
This is making a tablegen crash with a more friendly error.
Differential Revision: https://reviews.llvm.org/D109474
Added:
Modified:
mlir/include/mlir/TableGen/Operator.h
mlir/lib/TableGen/Operator.cpp
mlir/test/mlir-tblgen/op-error.td
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
Removed:
################################################################################
diff --git a/mlir/include/mlir/TableGen/Operator.h b/mlir/include/mlir/TableGen/Operator.h
index 4d41d4865b6dc..69e76d34b3713 100644
--- a/mlir/include/mlir/TableGen/Operator.h
+++ b/mlir/include/mlir/TableGen/Operator.h
@@ -64,6 +64,10 @@ class Operator {
// 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
index 03e5170e52688..57c957ff1c058 100644
--- a/mlir/lib/TableGen/Operator.cpp
+++ b/mlir/lib/TableGen/Operator.cpp
@@ -53,6 +53,7 @@ Operator::Operator(const llvm::Record &def)
cppNamespace = def.getValueAsString("cppNamespace");
populateOpStructure();
+ assertInvariants();
}
std::string Operator::getOperationName() const {
@@ -67,6 +68,41 @@ std::string Operator::getAdaptorName() const {
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<StringRef, StringRef> 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<int>(0, getNumOperands()))
+ checkName(getOperand(i).name, "operands");
+
+ // Check results amongst themselves and against operands.
+ for (int i : llvm::seq<int>(0, getNumResults()))
+ checkName(getResult(i).name, "results");
+
+ // Check regions amongst themselves and against operands and results.
+ for (int i : llvm::seq<int>(0, getNumRegions()))
+ checkName(getRegion(i).name, "regions");
+
+ // Check successors amongst themselves and against operands, results, and
+ // regions.
+ for (int i : llvm::seq<int>(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
index 587738e8f1116..4be846b9a940f 100644
--- 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 @@ def OpDefaultValueNotTrailing : Op<Test_Dialect, "default_value"> {
#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<Test_Dialect, "default_value"> {
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<Test_Dialect, "default_value"> {
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<Test_Dialect, "default_value"> {
+ 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<Test_Dialect, "default_value"> {
+ 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<Test_Dialect, "default_value"> {
+ 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<Test_Dialect, "default_value"> {
+ 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<Test_Dialect, "default_value"> {
+ 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<Test_Dialect, "default_value"> {
+ 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
index b9d63dc483cc1..e40fa9b8e6b7a 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -850,16 +850,10 @@ static void generateNamedOperandGetters(const Operator &op, Class &opClass,
// Then we emit nicer named getter methods by redirecting to the "sink" getter
// method.
- // Keep track of the operand names to find duplicates.
- SmallDenseSet<StringRef> 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 @@ void OpEmitter::genNamedResultGetters() {
m->body() << formatv(valueRangeReturnCode, "getOperation()->result_begin()",
"getODSResultIndexAndLength(index)");
- SmallDenseSet<StringRef> 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()
More information about the Mlir-commits
mailing list