[mlir] Change the default of `mlir-print-op-on-diagnostic` to true
Summary: It is a very common user trap to think that the location printed along with the diagnostic is the same as the current operation that caused the error. This revision changes the behavior to always print the current operation, except for when diagnostics are being verified. This is achieved by moving the command line flags in IR/ to be options on the MLIRContext.
Differential Revision: https://reviews.llvm.org/D77095
diff --git a/mlir/include/mlir/IR/MLIRContext.h b/mlir/include/mlir/IR/MLIRContext.h
index 6a65c4e..7df9b76 100644
--- a/mlir/include/mlir/IR/MLIRContext.h
+++ b/mlir/include/mlir/IR/MLIRContext.h
@@ -55,6 +55,22 @@
/// Enables creating operations in unregistered dialects.
void allowUnregisteredDialects(bool allow = true);
+ /// Return true if we should attach the operation to diagnostics emitted via
+ /// Operation::emit.
+ bool shouldPrintOpOnDiagnostic();
+
+ /// Set the flag specifying if we should attach the operation to diagnostics
+ /// emitted via Operation::emit.
+ void printOpOnDiagnostic(bool enable);
+
+ /// Return true if we should attach the current stacktrace to diagnostics when
+ /// emitted.
+ bool shouldPrintStackTraceOnDiagnostic();
+
+ /// Set the flag specifying if we should attach the current stacktrace when
+ /// emitting diagnostics.
+ void printStackTraceOnDiagnostic(bool enable);
+
/// Return information about all registered operations. This isn't very
/// efficient: typically you should ask the operations about their properties
/// directly.
diff --git a/mlir/lib/IR/Diagnostics.cpp b/mlir/lib/IR/Diagnostics.cpp
index 1acb7e7..f4e3017 100644
--- a/mlir/lib/IR/Diagnostics.cpp
+++ b/mlir/lib/IR/Diagnostics.cpp
@@ -16,7 +16,6 @@
#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringMap.h"
-#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Mutex.h"
#include "llvm/Support/PrettyStackTrace.h"
#include "llvm/Support/Regex.h"
@@ -27,11 +26,6 @@
using namespace mlir;
using namespace mlir::detail;
-static llvm::cl::opt<bool> printStackTraceOnDiagnostic(
- "mlir-print-stacktrace-on-diagnostic",
- llvm::cl::desc("When a diagnostic is emitted, also print the stack trace "
- "as an attached note"));
-
//===----------------------------------------------------------------------===//
// DiagnosticArgument
//===----------------------------------------------------------------------===//
@@ -278,13 +272,14 @@
/// diagnostic.
static InFlightDiagnostic
emitDiag(Location location, DiagnosticSeverity severity, const Twine &message) {
- auto &diagEngine = location->getContext()->getDiagEngine();
+ MLIRContext *ctx = location->getContext();
+ auto &diagEngine = ctx->getDiagEngine();
auto diag = diagEngine.emit(location, severity);
if (!message.isTriviallyEmpty())
diag << message;
// Add the stack trace as a note if necessary.
- if (printStackTraceOnDiagnostic) {
+ if (ctx->shouldPrintStackTraceOnDiagnostic()) {
std::string bt;
{
llvm::raw_string_ostream stream(bt);
diff --git a/mlir/lib/IR/MLIRContext.cpp b/mlir/lib/IR/MLIRContext.cpp
index 3875b57..2407c8f 100644
--- a/mlir/lib/IR/MLIRContext.cpp
+++ b/mlir/lib/IR/MLIRContext.cpp
@@ -31,6 +31,7 @@
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/Twine.h"
#include "llvm/Support/Allocator.h"
+#include "llvm/Support/CommandLine.h"
#include "llvm/Support/RWMutex.h"
#include "llvm/Support/raw_ostream.h"
#include <memory>
@@ -41,6 +42,17 @@
using llvm::hash_combine;
using llvm::hash_combine_range;
+static llvm::cl::opt<bool> clPrintOpOnDiagnostic(
+ "mlir-print-op-on-diagnostic",
+ llvm::cl::desc("When a diagnostic is emitted on an operation, also print "
+ "the operation as an attached note"),
+ llvm::cl::init(true));
+
+static llvm::cl::opt<bool> clPrintStackTraceOnDiagnostic(
+ "mlir-print-stacktrace-on-diagnostic",
+ llvm::cl::desc("When a diagnostic is emitted, also print the stack trace "
+ "as an attached note"));
+
/// A utility function to safely get or create a uniqued instance within the
/// given set container.
template <typename ValueT, typename DenseInfoT, typename KeyT,
@@ -170,6 +182,13 @@
/// detect such use cases
bool allowUnregisteredDialects = false;
+ /// If the operation should be attached to diagnostics printed via the
+ /// Operation::emit methods.
+ bool printOpOnDiagnostic;
+
+ /// If the current stack trace should be attached when emitting diagnostics.
+ bool printStackTraceOnDiagnostic;
+
//===--------------------------------------------------------------------===//
// Other
//===--------------------------------------------------------------------===//
@@ -234,7 +253,10 @@
UnknownLoc unknownLocAttr;
public:
- MLIRContextImpl() : identifiers(identifierAllocator) {}
+ MLIRContextImpl()
+ : printOpOnDiagnostic(clPrintOpOnDiagnostic),
+ printStackTraceOnDiagnostic(clPrintStackTraceOnDiagnostic),
+ identifiers(identifierAllocator) {}
};
} // end namespace mlir
@@ -366,6 +388,34 @@
impl->allowUnregisteredDialects = allowing;
}
+/// Return true if we should attach the operation to diagnostics emitted via
+/// Operation::emit.
+bool MLIRContext::shouldPrintOpOnDiagnostic() {
+ return impl->printOpOnDiagnostic;
+}
+
+/// Set the flag specifying if we should attach the operation to diagnostics
+/// emitted via Operation::emit.
+void MLIRContext::printOpOnDiagnostic(bool enable) {
+ // Let the command line option take priority.
+ if (!clPrintOpOnDiagnostic.getNumOccurrences())
+ impl->printOpOnDiagnostic = enable;
+}
+
+/// Return true if we should attach the current stacktrace to diagnostics when
+/// emitted.
+bool MLIRContext::shouldPrintStackTraceOnDiagnostic() {
+ return impl->printStackTraceOnDiagnostic;
+}
+
+/// Set the flag specifying if we should attach the current stacktrace when
+/// emitting diagnostics.
+void MLIRContext::printStackTraceOnDiagnostic(bool enable) {
+ // Let the command line option take priority.
+ if (!clPrintStackTraceOnDiagnostic.getNumOccurrences())
+ impl->printStackTraceOnDiagnostic = enable;
+}
+
/// Return information about all registered operations. This isn't very
/// efficient, typically you should ask the operations about their properties
/// directly.
diff --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp
index 58ebf29..b94ee96 100644
--- a/mlir/lib/IR/Operation.cpp
+++ b/mlir/lib/IR/Operation.cpp
@@ -13,16 +13,10 @@
#include "mlir/IR/PatternMatch.h"
#include "mlir/IR/StandardTypes.h"
#include "mlir/IR/TypeUtilities.h"
-#include "llvm/Support/CommandLine.h"
#include <numeric>
using namespace mlir;
-static llvm::cl::opt<bool> printOpOnDiagnostic(
- "mlir-print-op-on-diagnostic",
- llvm::cl::desc("When a diagnostic is emitted on an operation, also print "
- "the operation as an attached note"));
-
OpAsmParser::~OpAsmParser() {}
//===----------------------------------------------------------------------===//
@@ -252,7 +246,7 @@
/// any diagnostic handlers that may be listening.
InFlightDiagnostic Operation::emitError(const Twine &message) {
InFlightDiagnostic diag = mlir::emitError(getLoc(), message);
- if (printOpOnDiagnostic) {
+ if (getContext()->shouldPrintOpOnDiagnostic()) {
// Print out the operation explicitly here so that we can print the generic
// form.
// TODO(riverriddle) It would be nice if we could instead provide the
@@ -272,7 +266,7 @@
/// handlers that may be listening.
InFlightDiagnostic Operation::emitWarning(const Twine &message) {
InFlightDiagnostic diag = mlir::emitWarning(getLoc(), message);
- if (printOpOnDiagnostic)
+ if (getContext()->shouldPrintOpOnDiagnostic())
diag.attachNote(getLoc()) << "see current operation: " << *this;
return diag;
}
@@ -281,7 +275,7 @@
/// handlers that may be listening.
InFlightDiagnostic Operation::emitRemark(const Twine &message) {
InFlightDiagnostic diag = mlir::emitRemark(getLoc(), message);
- if (printOpOnDiagnostic)
+ if (getContext()->shouldPrintOpOnDiagnostic())
diag.attachNote(getLoc()) << "see current operation: " << *this;
return diag;
}
diff --git a/mlir/lib/Support/MlirOptMain.cpp b/mlir/lib/Support/MlirOptMain.cpp
index 53e336a..5c21e19 100644
--- a/mlir/lib/Support/MlirOptMain.cpp
+++ b/mlir/lib/Support/MlirOptMain.cpp
@@ -76,6 +76,7 @@
// Parse the input file.
MLIRContext context;
context.allowUnregisteredDialects(allowUnregisteredDialects);
+ context.printOpOnDiagnostic(!verifyDiagnostics);
// If we are in verify diagnostics mode then we have a lot of work to do,
// otherwise just perform the actions without worrying about it.
diff --git a/mlir/tools/mlir-translate/mlir-translate.cpp b/mlir/tools/mlir-translate/mlir-translate.cpp
index 1efcaec..d7c1eb6 100644
--- a/mlir/tools/mlir-translate/mlir-translate.cpp
+++ b/mlir/tools/mlir-translate/mlir-translate.cpp
@@ -74,6 +74,7 @@
raw_ostream &os) {
MLIRContext context;
context.allowUnregisteredDialects();
+ context.printOpOnDiagnostic(!verifyDiagnostics);
llvm::SourceMgr sourceMgr;
sourceMgr.AddNewSourceBuffer(std::move(ownedBuffer), llvm::SMLoc());
