[Mlir-commits] [mlir] 75f60c6 - [MLIR] Cleanup mlir-tblgen error messages for custom assembly formats.
Stephen Neuendorffer
llvmlistbot at llvm.org
Tue Apr 7 15:06:59 PDT 2020
Author: Stephen Neuendorffer
Date: 2020-04-07T15:06:50-07:00
New Revision: 75f60c698fdfc0ea46f84b7f92d667d6e7f53f7f
URL: https://github.com/llvm/llvm-project/commit/75f60c698fdfc0ea46f84b7f92d667d6e7f53f7f
DIFF: https://github.com/llvm/llvm-project/commit/75f60c698fdfc0ea46f84b7f92d667d6e7f53f7f.diff
LOG: [MLIR] Cleanup mlir-tblgen error messages for custom assembly formats.
The messages are somewhat cryptic, since they are not complete sentences,
include lots of ambiguous words, like 'format' which are hard to parse,
and include names from the users code which may, or may not make sense in
the context of the message. Start to clean this up and provide some
guidance for fixes.
Also, add a test for one of the messages which didn't have a test at all.
Differential Revision: https://reviews.llvm.org/D77449
Added:
Modified:
mlir/test/mlir-tblgen/op-format-spec.td
mlir/tools/mlir-tblgen/OpFormatGen.cpp
Removed:
################################################################################
diff --git a/mlir/test/mlir-tblgen/op-format-spec.td b/mlir/test/mlir-tblgen/op-format-spec.td
index 4e20b5ef4cf8..1e7a974272dc 100644
--- a/mlir/test/mlir-tblgen/op-format-spec.td
+++ b/mlir/test/mlir-tblgen/op-format-spec.td
@@ -19,7 +19,7 @@ class TestFormat_Op<string name, string fmt, list<OpTrait> traits = []>
//===----------------------------------------------------------------------===//
// attr-dict
-// CHECK: error: format missing 'attr-dict' directive
+// CHECK: error: 'attr-dict' directive not found in custom assembly format
def DirectiveAttrDictInvalidA : TestFormat_Op<"attrdict_invalid_a", [{
}]>;
// CHECK: error: 'attr-dict' directive has already been seen
@@ -295,26 +295,38 @@ def VariableInvalidK : TestFormat_Op<"variable_invalid_k", [{
// Coverage Checks
//===----------------------------------------------------------------------===//
-// CHECK: error: format missing instance of result #0('result') type
+// CHECK: error: type of result #0, named 'result', is not buildable and a buildable type cannot be inferred
+// CHECK: note: suggest adding a type constraint to the operation or adding a 'type($result)' directive to the custom assembly format
def ZCoverageInvalidA : TestFormat_Op<"variable_invalid_a", [{
attr-dict
}]>, Arguments<(ins AnyMemRef:$operand)>, Results<(outs AnyMemRef:$result)>;
-// CHECK: error: format missing instance of operand #0('operand')
+// CHECK: error: operand #0, named 'operand', not found in custom assembly format
+// CHECK: note: suggest adding a '$operand' directive to the custom assembly format
def ZCoverageInvalidB : TestFormat_Op<"variable_invalid_b", [{
type($result) attr-dict
}]>, Arguments<(ins AnyMemRef:$operand)>, Results<(outs AnyMemRef:$result)>;
-// CHECK: error: format missing instance of operand #0('operand') type
+// CHECK: error: type of operand #0, named 'operand', is not buildable and a buildable type cannot be inferred
+// CHECK: note: suggest adding a type constraint to the operation or adding a 'type($operand)' directive to the custom assembly format
def ZCoverageInvalidC : TestFormat_Op<"variable_invalid_c", [{
$operand type($result) attr-dict
}]>, Arguments<(ins AnyMemRef:$operand)>, Results<(outs AnyMemRef:$result)>;
-// CHECK: error: format missing instance of operand #0('operand') type
+// CHECK: error: type of operand #0, named 'operand', is not buildable and a buildable type cannot be inferred
+// CHECK: note: suggest adding a type constraint to the operation or adding a 'type($operand)' directive to the custom assembly format
def ZCoverageInvalidD : TestFormat_Op<"variable_invalid_d", [{
operands attr-dict
}]>, Arguments<(ins Variadic<I64>:$operand)>;
-// CHECK: error: format missing instance of result #0('result') type
+// CHECK: error: type of result #0, named 'result', is not buildable and a buildable type cannot be inferred
+// CHECK: note: suggest adding a type constraint to the operation or adding a 'type($result)' directive to the custom assembly format
def ZCoverageInvalidE : TestFormat_Op<"variable_invalid_e", [{
attr-dict
}]>, Results<(outs Variadic<I64>:$result)>;
+// CHECK: error: successor #0, named 'successor', not found in custom assembly format
+// CHECK: note: suggest adding a '$successor' directive to the custom assembly format
+def ZCoverageInvalidF : TestFormat_Op<"variable_invalid_f", [{
+ attr-dict
+}]> {
+ let successors = (successor AnySuccessor:$successor);
+}
// CHECK-NOT: error
def ZCoverageValidA : TestFormat_Op<"variable_valid_a", [{
$operand type($operand) type($result) attr-dict
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index fdf8f0265509..e4b66948d27a 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -1064,6 +1064,8 @@ class FormatLexer {
Token emitError(llvm::SMLoc loc, const Twine &msg);
Token emitError(const char *loc, const Twine &msg);
+ Token emitErrorAndNote(llvm::SMLoc loc, const Twine &msg, const Twine ¬e);
+
private:
Token formToken(Token::Kind kind, const char *tokStart) {
return Token(kind, StringRef(tokStart, curPtr - tokStart));
@@ -1092,6 +1094,12 @@ Token FormatLexer::emitError(llvm::SMLoc loc, const Twine &msg) {
srcMgr.PrintMessage(loc, llvm::SourceMgr::DK_Error, msg);
return formToken(Token::error, loc.getPointer());
}
+Token FormatLexer::emitErrorAndNote(llvm::SMLoc loc, const Twine &msg,
+ const Twine ¬e) {
+ srcMgr.PrintMessage(loc, llvm::SourceMgr::DK_Error, msg);
+ srcMgr.PrintMessage(loc, llvm::SourceMgr::DK_Note, note);
+ return formToken(Token::error, loc.getPointer());
+}
Token FormatLexer::emitError(const char *loc, const Twine &msg) {
return emitError(llvm::SMLoc::getFromPointer(loc), msg);
}
@@ -1325,6 +1333,11 @@ class FormatParser {
lexer.emitError(loc, msg);
return failure();
}
+ LogicalResult emitErrorAndNote(llvm::SMLoc loc, const Twine &msg,
+ const Twine ¬e) {
+ lexer.emitErrorAndNote(loc, msg, note);
+ return failure();
+ }
//===--------------------------------------------------------------------===//
// Fields
@@ -1360,7 +1373,8 @@ LogicalResult FormatParser::parse() {
// Check that the attribute dictionary is in the format.
if (!hasAttrDict)
- return emitError(loc, "format missing 'attr-dict' directive");
+ return emitError(loc, "'attr-dict' directive not found in "
+ "custom assembly format");
// Check for any type traits that we can use for inferring types.
llvm::StringMap<TypeResolutionInstance> variableTyResolver;
@@ -1465,8 +1479,12 @@ LogicalResult FormatParser::verifyOperands(
// Check that the operand itself is in the format.
if (!hasAllOperands && !seenOperands.count(&operand)) {
- return emitError(loc, "format missing instance of operand #" + Twine(i) +
- "('" + operand.name + "')");
+ return emitErrorAndNote(loc,
+ "operand #" + Twine(i) + ", named '" +
+ operand.name +
+ "', not found in custom assembly format",
+ "suggest adding a '$" + operand.name +
+ "' directive to the custom assembly format");
}
// Check that the operand type is in the format, or that it can be inferred.
@@ -1485,8 +1503,15 @@ LogicalResult FormatParser::verifyOperands(
// we aren't using the 'operands' directive.
Optional<StringRef> builder = operand.constraint.getBuilderCall();
if (!builder || (hasAllOperands && operand.isVariadic())) {
- return emitError(loc, "format missing instance of operand #" + Twine(i) +
- "('" + operand.name + "') type");
+ return emitErrorAndNote(
+ loc,
+ "type of operand #" + Twine(i) + ", named '" + operand.name +
+ "', is not buildable and a buildable " +
+ "type cannot be inferred",
+ "suggest adding a type constraint "
+ "to the operation or adding a "
+ "'type($" +
+ operand.name + ")' directive to the " + "custom assembly format");
}
auto it = buildableTypes.insert({*builder, buildableTypes.size()});
fmt.operandTypes[i].setBuilderIdx(it.first->second);
@@ -1520,8 +1545,15 @@ LogicalResult FormatParser::verifyResults(
NamedTypeConstraint &result = op.getResult(i);
Optional<StringRef> builder = result.constraint.getBuilderCall();
if (!builder || result.constraint.isVariadic()) {
- return emitError(loc, "format missing instance of result #" + Twine(i) +
- "('" + result.name + "') type");
+ return emitErrorAndNote(
+ loc,
+ "type of result #" + Twine(i) + ", named '" + result.name +
+ "', is not buildable and a buildable " +
+ "type cannot be inferred",
+ "suggest adding a type constraint "
+ "to the operation or adding a "
+ "'type($" +
+ result.name + ")' directive to the " + "custom assembly format");
}
// Note in the format that this result uses the custom builder.
auto it = buildableTypes.insert({*builder, buildableTypes.size()});
@@ -1538,8 +1570,12 @@ LogicalResult FormatParser::verifySuccessors(llvm::SMLoc loc) {
for (unsigned i = 0, e = op.getNumSuccessors(); i != e; ++i) {
const NamedSuccessor &successor = op.getSuccessor(i);
if (!seenSuccessors.count(&successor)) {
- return emitError(loc, "format missing instance of successor #" +
- Twine(i) + "('" + successor.name + "')");
+ return emitErrorAndNote(loc,
+ "successor #" + Twine(i) + ", named '" +
+ successor.name +
+ "', not found in custom assembly format",
+ "suggest adding a '$" + successor.name +
+ "' directive to the custom assembly format");
}
}
return success();
More information about the Mlir-commits
mailing list