diff --git a/mlir/examples/transform-opt/mlir-transform-opt.cpp b/mlir/examples/transform-opt/mlir-transform-opt.cpp index 10e16096211a..2fec9be92e6e 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 verifyDiagnostics{ - "verify-diagnostics", - cl::desc("Check that emitted diagnostics match expected-* lines " - "on the corresponding line"), - cl::init(false)}; + cl::opt 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 payloadFilename{cl::Positional, cl::desc(""), cl::init("-")}; @@ -102,12 +113,17 @@ public: /// 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 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 + verifyDiagnostics) : verifyDiagnostics(verifyDiagnostics) {} /// Deconstructs the source manager. Note that `checkResults` must have been @@ -179,7 +197,8 @@ public: // 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 @@ public: private: /// Indicates whether diagnostic message verification is requested. - const bool verifyDiagnostics; + const std::optional + 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 payloadRoot = diff --git a/mlir/include/mlir/IR/Diagnostics.h b/mlir/include/mlir/IR/Diagnostics.h index 59bed71e4db8..4ed042390213 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 af379797fe86..94231227599c 100644 --- a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h +++ b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h @@ -185,11 +185,20 @@ public: /// 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 @@ protected: /// 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 b699e396f657..59d803035bda 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> @@ -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 2924a1205f57..31e0caa76811 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 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 + 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 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 56773f599d5c..f2a81cca2abe 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 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 + 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 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(); 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 881260ce60d3..c355fd071bdb 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@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 000000000000..de29693604b4 --- /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@+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 000000000000..0efeaa84a3f4 --- /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"() : () -> () +}