[Mlir-commits] [mlir] 1cec5ff - [mlir] implement `-verify-diagnostics=only-expected` (#135131)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Thu Apr 10 15:50:03 PDT 2025
Author: Maksim Levental
Date: 2025-04-10T18:50:00-04:00
New Revision: 1cec5fffd8fddd9d85b516f876093b0e3f0eec5f
URL: https://github.com/llvm/llvm-project/commit/1cec5fffd8fddd9d85b516f876093b0e3f0eec5f
DIFF: https://github.com/llvm/llvm-project/commit/1cec5fffd8fddd9d85b516f876093b0e3f0eec5f.diff
LOG: [mlir] implement `-verify-diagnostics=only-expected` (#135131)
This PR implements `verify-diagnostics=only-expected` which is a "best
effort" verification - i.e., `unexpected`s and `near-misses` will not be
considered failures. The purpose is to enable narrowly scoped checking
of verification remarks (just as we have for lit where only a subset of
lines get `CHECK`ed).
Added:
mlir/test/Pass/full_diagnostics_only_expected.mlir
mlir/test/mlir-translate/verify-only-expected.mlir
Modified:
mlir/examples/transform-opt/mlir-transform-opt.cpp
mlir/include/mlir/IR/Diagnostics.h
mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
mlir/lib/IR/Diagnostics.cpp
mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp
mlir/test/Pass/full_diagnostics.mlir
Removed:
################################################################################
diff --git a/mlir/examples/transform-opt/mlir-transform-opt.cpp b/mlir/examples/transform-opt/mlir-transform-opt.cpp
index 10e16096211ad..2fec9be92e6e3 100644
--- a/mlir/examples/transform-opt/mlir-transform-opt.cpp
+++ b/mlir/examples/transform-opt/mlir-transform-opt.cpp
@@ -38,11 +38,22 @@ struct MlirTransformOptCLOptions {
cl::desc("Allow operations coming from an unregistered dialect"),
cl::init(false)};
- cl::opt<bool> verifyDiagnostics{
- "verify-diagnostics",
- cl::desc("Check that emitted diagnostics match expected-* lines "
- "on the corresponding line"),
- cl::init(false)};
+ cl::opt<mlir::SourceMgrDiagnosticVerifierHandler::Level> verifyDiagnostics{
+ "verify-diagnostics", llvm::cl::ValueOptional,
+ cl::desc("Check that emitted diagnostics match expected-* lines on the "
+ "corresponding line"),
+ cl::values(
+ clEnumValN(
+ mlir::SourceMgrDiagnosticVerifierHandler::Level::All, "all",
+ "Check all diagnostics (expected, unexpected, near-misses)"),
+ // Implicit value: when passed with no arguments, e.g.
+ // `--verify-diagnostics` or `--verify-diagnostics=`.
+ clEnumValN(
+ mlir::SourceMgrDiagnosticVerifierHandler::Level::All, "",
+ "Check all diagnostics (expected, unexpected, near-misses)"),
+ clEnumValN(
+ mlir::SourceMgrDiagnosticVerifierHandler::Level::OnlyExpected,
+ "only-expected", "Check only expected diagnostics"))};
cl::opt<std::string> payloadFilename{cl::Positional, cl::desc("<input file>"),
cl::init("-")};
@@ -102,12 +113,17 @@ class DiagnosticHandlerWrapper {
/// Constructs the diagnostic handler of the specified kind of the given
/// source manager and context.
- DiagnosticHandlerWrapper(Kind kind, llvm::SourceMgr &mgr,
- mlir::MLIRContext *context) {
- if (kind == Kind::EmitDiagnostics)
+ DiagnosticHandlerWrapper(
+ Kind kind, llvm::SourceMgr &mgr, mlir::MLIRContext *context,
+ std::optional<mlir::SourceMgrDiagnosticVerifierHandler::Level> level =
+ {}) {
+ if (kind == Kind::EmitDiagnostics) {
handler = new mlir::SourceMgrDiagnosticHandler(mgr, context);
- else
- handler = new mlir::SourceMgrDiagnosticVerifierHandler(mgr, context);
+ } else {
+ assert(level.has_value() && "expected level");
+ handler =
+ new mlir::SourceMgrDiagnosticVerifierHandler(mgr, context, *level);
+ }
}
/// This object is non-copyable but movable.
@@ -150,7 +166,9 @@ class TransformSourceMgr {
public:
/// Constructs the source manager indicating whether diagnostic messages will
/// be verified later on.
- explicit TransformSourceMgr(bool verifyDiagnostics)
+ explicit TransformSourceMgr(
+ std::optional<mlir::SourceMgrDiagnosticVerifierHandler::Level>
+ verifyDiagnostics)
: verifyDiagnostics(verifyDiagnostics) {}
/// Deconstructs the source manager. Note that `checkResults` must have been
@@ -179,7 +197,8 @@ class TransformSourceMgr {
// verification needs to happen and store it.
if (verifyDiagnostics) {
diagHandlers.emplace_back(
- DiagnosticHandlerWrapper::Kind::VerifyDiagnostics, mgr, &context);
+ DiagnosticHandlerWrapper::Kind::VerifyDiagnostics, mgr, &context,
+ verifyDiagnostics);
} else {
diagHandlers.emplace_back(DiagnosticHandlerWrapper::Kind::EmitDiagnostics,
mgr, &context);
@@ -204,7 +223,8 @@ class TransformSourceMgr {
private:
/// Indicates whether diagnostic message verification is requested.
- const bool verifyDiagnostics;
+ const std::optional<mlir::SourceMgrDiagnosticVerifierHandler::Level>
+ verifyDiagnostics;
/// Indicates that diagnostic message verification has taken place, and the
/// deconstruction is therefore safe.
@@ -248,7 +268,9 @@ static llvm::LogicalResult processPayloadBuffer(
context.allowUnregisteredDialects(clOptions->allowUnregisteredDialects);
mlir::ParserConfig config(&context);
TransformSourceMgr sourceMgr(
- /*verifyDiagnostics=*/clOptions->verifyDiagnostics);
+ /*verifyDiagnostics=*/clOptions->verifyDiagnostics.getNumOccurrences()
+ ? std::optional{clOptions->verifyDiagnostics.getValue()}
+ : std::nullopt);
// Parse the input buffer that will be used as transform payload.
mlir::OwningOpRef<mlir::Operation *> payloadRoot =
diff --git a/mlir/include/mlir/IR/Diagnostics.h b/mlir/include/mlir/IR/Diagnostics.h
index 59bed71e4db88..4ed0423902138 100644
--- a/mlir/include/mlir/IR/Diagnostics.h
+++ b/mlir/include/mlir/IR/Diagnostics.h
@@ -626,9 +626,12 @@ struct SourceMgrDiagnosticVerifierHandlerImpl;
/// corresponding line of the source file.
class SourceMgrDiagnosticVerifierHandler : public SourceMgrDiagnosticHandler {
public:
+ enum class Level { None = 0, All, OnlyExpected };
SourceMgrDiagnosticVerifierHandler(llvm::SourceMgr &srcMgr, MLIRContext *ctx,
- raw_ostream &out);
- SourceMgrDiagnosticVerifierHandler(llvm::SourceMgr &srcMgr, MLIRContext *ctx);
+ raw_ostream &out,
+ Level level = Level::All);
+ SourceMgrDiagnosticVerifierHandler(llvm::SourceMgr &srcMgr, MLIRContext *ctx,
+ Level level = Level::All);
~SourceMgrDiagnosticVerifierHandler();
/// Returns the status of the handler and verifies that all expected
diff --git a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
index af379797fe865..94231227599c9 100644
--- a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
+++ b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
@@ -185,11 +185,20 @@ class MlirOptMainConfig {
/// Set whether to check that emitted diagnostics match `expected-*` lines on
/// the corresponding line. This is meant for implementing diagnostic tests.
- MlirOptMainConfig &verifyDiagnostics(bool verify) {
+ MlirOptMainConfig &
+ verifyDiagnostics(SourceMgrDiagnosticVerifierHandler::Level verify) {
verifyDiagnosticsFlag = verify;
return *this;
}
- bool shouldVerifyDiagnostics() const { return verifyDiagnosticsFlag; }
+
+ bool shouldVerifyDiagnostics() const {
+ return verifyDiagnosticsFlag !=
+ SourceMgrDiagnosticVerifierHandler::Level::None;
+ }
+
+ SourceMgrDiagnosticVerifierHandler::Level verifyDiagnosticsLevel() const {
+ return verifyDiagnosticsFlag;
+ }
/// Set whether to run the verifier after each transformation pass.
MlirOptMainConfig &verifyPasses(bool verify) {
@@ -276,7 +285,8 @@ class MlirOptMainConfig {
/// Set whether to check that emitted diagnostics match `expected-*` lines on
/// the corresponding line. This is meant for implementing diagnostic tests.
- bool verifyDiagnosticsFlag = false;
+ SourceMgrDiagnosticVerifierHandler::Level verifyDiagnosticsFlag =
+ SourceMgrDiagnosticVerifierHandler::Level::None;
/// Run the verifier after each transformation pass.
bool verifyPassesFlag = true;
diff --git a/mlir/lib/IR/Diagnostics.cpp b/mlir/lib/IR/Diagnostics.cpp
index b699e396f6577..59d803035bda0 100644
--- a/mlir/lib/IR/Diagnostics.cpp
+++ b/mlir/lib/IR/Diagnostics.cpp
@@ -661,7 +661,9 @@ struct ExpectedDiag {
};
struct SourceMgrDiagnosticVerifierHandlerImpl {
- SourceMgrDiagnosticVerifierHandlerImpl() : status(success()) {}
+ SourceMgrDiagnosticVerifierHandlerImpl(
+ SourceMgrDiagnosticVerifierHandler::Level level)
+ : status(success()), level(level) {}
/// Returns the expected diagnostics for the given source file.
std::optional<MutableArrayRef<ExpectedDiag>>
@@ -672,6 +674,10 @@ struct SourceMgrDiagnosticVerifierHandlerImpl {
computeExpectedDiags(raw_ostream &os, llvm::SourceMgr &mgr,
const llvm::MemoryBuffer *buf);
+ SourceMgrDiagnosticVerifierHandler::Level getVerifyLevel() const {
+ return level;
+ }
+
/// The current status of the verifier.
LogicalResult status;
@@ -685,6 +691,10 @@ struct SourceMgrDiagnosticVerifierHandlerImpl {
llvm::Regex expected =
llvm::Regex("expected-(error|note|remark|warning)(-re)? "
"*(@([+-][0-9]+|above|below|unknown))? *{{(.*)}}$");
+
+ /// Verification level.
+ SourceMgrDiagnosticVerifierHandler::Level level =
+ SourceMgrDiagnosticVerifierHandler::Level::All;
};
} // namespace detail
} // namespace mlir
@@ -803,9 +813,9 @@ SourceMgrDiagnosticVerifierHandlerImpl::computeExpectedDiags(
}
SourceMgrDiagnosticVerifierHandler::SourceMgrDiagnosticVerifierHandler(
- llvm::SourceMgr &srcMgr, MLIRContext *ctx, raw_ostream &out)
+ llvm::SourceMgr &srcMgr, MLIRContext *ctx, raw_ostream &out, Level level)
: SourceMgrDiagnosticHandler(srcMgr, ctx, out),
- impl(new SourceMgrDiagnosticVerifierHandlerImpl()) {
+ impl(new SourceMgrDiagnosticVerifierHandlerImpl(level)) {
// Compute the expected diagnostics for each of the current files in the
// source manager.
for (unsigned i = 0, e = mgr.getNumBuffers(); i != e; ++i)
@@ -823,8 +833,8 @@ SourceMgrDiagnosticVerifierHandler::SourceMgrDiagnosticVerifierHandler(
}
SourceMgrDiagnosticVerifierHandler::SourceMgrDiagnosticVerifierHandler(
- llvm::SourceMgr &srcMgr, MLIRContext *ctx)
- : SourceMgrDiagnosticVerifierHandler(srcMgr, ctx, llvm::errs()) {}
+ llvm::SourceMgr &srcMgr, MLIRContext *ctx, Level level)
+ : SourceMgrDiagnosticVerifierHandler(srcMgr, ctx, llvm::errs(), level) {}
SourceMgrDiagnosticVerifierHandler::~SourceMgrDiagnosticVerifierHandler() {
// Ensure that all expected diagnostics were handled.
@@ -898,6 +908,9 @@ void SourceMgrDiagnosticVerifierHandler::process(LocationAttr loc,
}
}
+ if (impl->getVerifyLevel() == Level::OnlyExpected)
+ return;
+
// Otherwise, emit an error for the near miss.
if (nearMiss)
mgr.PrintMessage(os, nearMiss->fileLoc, llvm::SourceMgr::DK_Error,
diff --git a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
index 2924a1205f574..31e0caa768113 100644
--- a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
+++ b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
@@ -163,11 +163,26 @@ struct MlirOptMainConfigCLOptions : public MlirOptMainConfig {
cl::desc("Split marker to use for merging the ouput"),
cl::location(outputSplitMarkerFlag), cl::init(kDefaultSplitMarker));
- static cl::opt<bool, /*ExternalStorage=*/true> verifyDiagnostics(
- "verify-diagnostics",
- cl::desc("Check that emitted diagnostics match "
- "expected-* lines on the corresponding line"),
- cl::location(verifyDiagnosticsFlag), cl::init(false));
+ static cl::opt<SourceMgrDiagnosticVerifierHandler::Level,
+ /*ExternalStorage=*/true>
+ verifyDiagnostics{
+ "verify-diagnostics", llvm::cl::ValueOptional,
+ cl::desc("Check that emitted diagnostics match expected-* lines on "
+ "the corresponding line"),
+ cl::location(verifyDiagnosticsFlag),
+ cl::values(
+ clEnumValN(SourceMgrDiagnosticVerifierHandler::Level::All,
+ "all",
+ "Check all diagnostics (expected, unexpected, "
+ "near-misses)"),
+ // Implicit value: when passed with no arguments, e.g.
+ // `--verify-diagnostics` or `--verify-diagnostics=`.
+ clEnumValN(SourceMgrDiagnosticVerifierHandler::Level::All, "",
+ "Check all diagnostics (expected, unexpected, "
+ "near-misses)"),
+ clEnumValN(
+ SourceMgrDiagnosticVerifierHandler::Level::OnlyExpected,
+ "only-expected", "Check only expected diagnostics"))};
static cl::opt<bool, /*ExternalStorage=*/true> verifyPasses(
"verify-each",
@@ -537,7 +552,8 @@ static LogicalResult processBuffer(raw_ostream &os,
return performActions(os, sourceMgr, &context, config);
}
- SourceMgrDiagnosticVerifierHandler sourceMgrHandler(*sourceMgr, &context);
+ SourceMgrDiagnosticVerifierHandler sourceMgrHandler(
+ *sourceMgr, &context, config.verifyDiagnosticsLevel());
// Do any processing requested by command line flags. We don't care whether
// these actions succeed or fail, we only care what diagnostics they produce
diff --git a/mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp b/mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp
index 56773f599d5ce..f2a81cca2abe5 100644
--- a/mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp
+++ b/mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp
@@ -72,11 +72,23 @@ LogicalResult mlir::mlirTranslateMain(int argc, char **argv,
"default marker and process each chunk independently"),
llvm::cl::init("")};
- static llvm::cl::opt<bool> verifyDiagnostics(
- "verify-diagnostics",
- llvm::cl::desc("Check that emitted diagnostics match "
- "expected-* lines on the corresponding line"),
- llvm::cl::init(false));
+ static llvm::cl::opt<SourceMgrDiagnosticVerifierHandler::Level>
+ verifyDiagnostics{
+ "verify-diagnostics", llvm::cl::ValueOptional,
+ llvm::cl::desc("Check that emitted diagnostics match expected-* "
+ "lines on the corresponding line"),
+ llvm::cl::values(
+ clEnumValN(
+ SourceMgrDiagnosticVerifierHandler::Level::All, "all",
+ "Check all diagnostics (expected, unexpected, near-misses)"),
+ // Implicit value: when passed with no arguments, e.g.
+ // `--verify-diagnostics` or `--verify-diagnostics=`.
+ clEnumValN(
+ SourceMgrDiagnosticVerifierHandler::Level::All, "",
+ "Check all diagnostics (expected, unexpected, near-misses)"),
+ clEnumValN(
+ SourceMgrDiagnosticVerifierHandler::Level::OnlyExpected,
+ "only-expected", "Check only expected diagnostics"))};
static llvm::cl::opt<bool> errorDiagnosticsOnly(
"error-diagnostics-only",
@@ -149,17 +161,17 @@ LogicalResult mlir::mlirTranslateMain(int argc, char **argv,
MLIRContext context;
context.allowUnregisteredDialects(allowUnregisteredDialects);
- context.printOpOnDiagnostic(!verifyDiagnostics);
+ context.printOpOnDiagnostic(verifyDiagnostics.getNumOccurrences() == 0);
auto sourceMgr = std::make_shared<llvm::SourceMgr>();
sourceMgr->AddNewSourceBuffer(std::move(ownedBuffer), SMLoc());
- if (verifyDiagnostics) {
+ if (verifyDiagnostics.getNumOccurrences()) {
// In the diagnostic verification flow, we ignore whether the
// translation failed (in most cases, it is expected to fail) and we do
// not filter non-error diagnostics even if `errorDiagnosticsOnly` is
// set. Instead, we check if the diagnostics were produced as expected.
- SourceMgrDiagnosticVerifierHandler sourceMgrHandler(*sourceMgr,
- &context);
+ SourceMgrDiagnosticVerifierHandler sourceMgrHandler(
+ *sourceMgr, &context, verifyDiagnostics);
(void)(*translationRequested)(sourceMgr, os, &context);
result = sourceMgrHandler.verify();
} else if (errorDiagnosticsOnly) {
diff --git a/mlir/test/Pass/full_diagnostics.mlir b/mlir/test/Pass/full_diagnostics.mlir
index 881260ce60d32..c355fd071bdb6 100644
--- a/mlir/test/Pass/full_diagnostics.mlir
+++ b/mlir/test/Pass/full_diagnostics.mlir
@@ -1,4 +1,5 @@
// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(test-pass-failure{gen-diagnostics}))' -verify-diagnostics
+// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(test-pass-failure{gen-diagnostics}))' -verify-diagnostics=all
// Test that all errors are reported.
// expected-error at below {{illegal operation}}
diff --git a/mlir/test/Pass/full_diagnostics_only_expected.mlir b/mlir/test/Pass/full_diagnostics_only_expected.mlir
new file mode 100644
index 0000000000000..de29693604b4f
--- /dev/null
+++ b/mlir/test/Pass/full_diagnostics_only_expected.mlir
@@ -0,0 +1,17 @@
+// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(test-pass-failure{gen-diagnostics}))' -verify-diagnostics=only-expected
+
+// Test that only expected errors are reported.
+// reports {{illegal operation}} but unchecked
+func.func @TestAlwaysIllegalOperationPass1() {
+ return
+}
+
+// expected-error at +1 {{illegal operation}}
+func.func @TestAlwaysIllegalOperationPass2() {
+ return
+}
+
+// reports {{illegal operation}} but unchecked
+func.func @TestAlwaysIllegalOperationPass3() {
+ return
+}
diff --git a/mlir/test/mlir-translate/verify-only-expected.mlir b/mlir/test/mlir-translate/verify-only-expected.mlir
new file mode 100644
index 0000000000000..0efeaa84a3f4f
--- /dev/null
+++ b/mlir/test/mlir-translate/verify-only-expected.mlir
@@ -0,0 +1,24 @@
+// Check that verify-diagnostics=only-expected passes with only one actual `expected-error`
+// RUN: mlir-translate %s --allow-unregistered-dialect -verify-diagnostics=only-expected -split-input-file -mlir-to-llvmir
+
+// Check that verify-diagnostics=all fails because we're missing two `expected-error`
+// RUN: not mlir-translate %s --allow-unregistered-dialect -verify-diagnostics=all -split-input-file -mlir-to-llvmir 2>&1 | FileCheck %s --check-prefix=CHECK-VERIFY-ALL
+// CHECK-VERIFY-ALL: unexpected error: cannot be converted to LLVM IR: missing `LLVMTranslationDialectInterface` registration for dialect for op: simple.terminator1
+// CHECK-VERIFY-ALL: unexpected error: cannot be converted to LLVM IR: missing `LLVMTranslationDialectInterface` registration for dialect for op: simple.terminator3
+
+llvm.func @trivial() {
+ "simple.terminator1"() : () -> ()
+}
+
+// -----
+
+llvm.func @trivial() {
+ // expected-error @+1 {{cannot be converted to LLVM IR: missing `LLVMTranslationDialectInterface` registration for dialect for op: simple.terminator2}}
+ "simple.terminator2"() : () -> ()
+}
+
+// -----
+
+llvm.func @trivial() {
+ "simple.terminator3"() : () -> ()
+}
More information about the Mlir-commits
mailing list