diff --git a/mlir/docs/PassManagement.md b/mlir/docs/PassManagement.md --- a/mlir/docs/PassManagement.md +++ b/mlir/docs/PassManagement.md @@ -992,3 +992,28 @@ } } ``` + +### Minimal Reproducer Generation + +An additional flag may be passed to +`PassManager::enableCrashReproducerGeneration`, and specified via +`pass-pipeline-minimal-reproducer` on the command line, that signals that the +pass manager should attempt to generate a "minimal" reproducer. This will +attempt to generate a reproducer containing IR right before the pass that fails. +This is useful for situations where the crash is known to be within a specific +pass, or when the original input relies on components(like dialects or passes) +that may not always be available. + +For example, if the failure in the previous example came from `canonicalize` the +following reproducer will be generated: + +```mlir +// configuration: -pass-pipeline='func(canonicalize)' +// note: verifyPasses=false + +module { + func @foo() { + ... + } +} +``` diff --git a/mlir/include/mlir/Pass/Pass.h b/mlir/include/mlir/Pass/Pass.h --- a/mlir/include/mlir/Pass/Pass.h +++ b/mlir/include/mlir/Pass/Pass.h @@ -252,6 +252,9 @@ /// Allow access to 'clone' and 'run'. friend class OpPassManager; + /// Allow access to 'run'. + friend class PassManager; + /// Allow access to 'passOptions'. friend class PassInfo; }; diff --git a/mlir/include/mlir/Pass/PassManager.h b/mlir/include/mlir/Pass/PassManager.h --- a/mlir/include/mlir/Pass/PassManager.h +++ b/mlir/include/mlir/Pass/PassManager.h @@ -106,6 +106,9 @@ /// Allow access to the constructor. friend class PassManager; + + /// Allow access. + friend detail::OpPassManagerImpl; }; //===----------------------------------------------------------------------===// @@ -145,8 +148,10 @@ /// Enable support for the pass manager to generate a reproducer on the event /// of a crash or a pass failure. `outputFile` is a .mlir filename used to - /// write the generated reproducer. - void enableCrashReproducerGeneration(StringRef outputFile); + /// write the generated reproducer. If `genMinimalReproducer` is true, the + /// pass manager will attempt to generate a minimal reproducer. + void enableCrashReproducerGeneration(StringRef outputFile, + bool genMinimalReproducer = false); //===--------------------------------------------------------------------===// // Instrumentations @@ -245,8 +250,12 @@ /// Dump the statistics of the passes within this pass manager. void dumpStatistics(); - /// Flag that specifies if pass timing is enabled. - bool passTiming : 1; + /// Run the pass manager with crash recover enabled. + LogicalResult runWithCrashRecovery(ModuleOp module, AnalysisManager am); + /// Run the given passes with crash recover enabled. + LogicalResult + runWithCrashRecovery(MutableArrayRef> passes, + ModuleOp module, AnalysisManager am); /// Flag that specifies if pass statistics should be dumped. Optional passStatisticsMode; @@ -256,6 +265,12 @@ /// An optional filename to use when generating a crash reproducer if valid. Optional crashReproducerFileName; + + /// Flag that specifies if pass timing is enabled. + bool passTiming : 1; + + /// Flag that specifies if the generated crash reproducer should be minimal. + bool minimalReproducer : 1; }; /// Register a set of useful command-line options that can be used to configure diff --git a/mlir/lib/Pass/Pass.cpp b/mlir/lib/Pass/Pass.cpp --- a/mlir/lib/Pass/Pass.cpp +++ b/mlir/lib/Pass/Pass.cpp @@ -121,17 +121,32 @@ } /// Merge the passes of this pass manager into the one provided. - void mergeInto(OpPassManagerImpl &rhs) { - assert(name == rhs.name && "merging unrelated pass managers"); - for (auto &pass : passes) - rhs.passes.push_back(std::move(pass)); - passes.clear(); + void mergeInto(OpPassManagerImpl &rhs); + + /// Nest a new operation pass manager for the given operation kind under this + /// pass manager. + OpPassManager &nest(const OperationName &nestedName); + OpPassManager &nest(StringRef nestedName) { + return nest(OperationName(nestedName, getContext())); } + /// Add the given pass to this pass manager. If this pass has a concrete + /// operation type, it must be the same type as this pass manager. + void addPass(std::unique_ptr pass); + /// Coalesce adjacent AdaptorPasses into one large adaptor. This runs /// recursively through the pipeline graph. void coalesceAdjacentAdaptorPasses(); + /// Split all of AdaptorPasses such that each adaptor only contains one leaf + /// pass. + void splitAdaptorPasses(); + + /// Return an instance of the context. + MLIRContext *getContext() const { + return name.getAbstractOperation()->dialect.getContext(); + } + /// The name of the operation that passes of this pass manager operate on. OperationName name; @@ -147,8 +162,32 @@ } // end namespace detail } // end namespace mlir -/// Coalesce adjacent AdaptorPasses into one large adaptor. This runs -/// recursively through the pipeline graph. +void OpPassManagerImpl::mergeInto(OpPassManagerImpl &rhs) { + assert(name == rhs.name && "merging unrelated pass managers"); + for (auto &pass : passes) + rhs.passes.push_back(std::move(pass)); + passes.clear(); +} + +OpPassManager &OpPassManagerImpl::nest(const OperationName &nestedName) { + OpPassManager nested(nestedName, disableThreads, verifyPasses); + auto *adaptor = new OpToOpPassAdaptor(std::move(nested)); + addPass(std::unique_ptr(adaptor)); + return adaptor->getPassManagers().front(); +} + +void OpPassManagerImpl::addPass(std::unique_ptr pass) { + // If this pass runs on a different operation than this pass manager, then + // implicitly nest a pass manager for this operation. + auto passOpName = pass->getOpName(); + if (passOpName && passOpName != name.getStringRef()) + return nest(*passOpName).addPass(std::move(pass)); + + passes.emplace_back(std::move(pass)); + if (verifyPasses) + passes.emplace_back(std::make_unique()); +} + void OpPassManagerImpl::coalesceAdjacentAdaptorPasses() { // Bail out early if there are no adaptor passes. if (llvm::none_of(passes, [](std::unique_ptr &pass) { @@ -199,6 +238,31 @@ llvm::erase_if(passes, std::logical_not>()); } +void OpPassManagerImpl::splitAdaptorPasses() { + std::vector> oldPasses; + std::swap(passes, oldPasses); + + for (std::unique_ptr &pass : oldPasses) { + // If this pass isn't an adaptor, move it directly to the new pass list. + auto *currentAdaptor = dyn_cast(pass.get()); + if (!currentAdaptor) { + passes.push_back(std::move(pass)); + continue; + } + + // Otherwise, split the adaptors of each manager within the adaptor. + for (OpPassManager &adaptorPM : currentAdaptor->getPassManagers()) { + adaptorPM.getImpl().splitAdaptorPasses(); + + // Add all non-verifier passes to this pass manager. + for (std::unique_ptr &nestedPass : adaptorPM.getImpl().passes) { + if (!isa(nestedPass.get())) + nest(adaptorPM.getOpName()).addPass(std::move(nestedPass)); + } + } + } +} + //===----------------------------------------------------------------------===// // OpPassManager //===----------------------------------------------------------------------===// @@ -242,27 +306,16 @@ /// Nest a new operation pass manager for the given operation kind under this /// pass manager. OpPassManager &OpPassManager::nest(const OperationName &nestedName) { - OpPassManager nested(nestedName, impl->disableThreads, impl->verifyPasses); - auto *adaptor = new OpToOpPassAdaptor(std::move(nested)); - addPass(std::unique_ptr(adaptor)); - return adaptor->getPassManagers().front(); + return impl->nest(nestedName); } OpPassManager &OpPassManager::nest(StringRef nestedName) { - return nest(OperationName(nestedName, getContext())); + return impl->nest(nestedName); } /// Add the given pass to this pass manager. If this pass has a concrete /// operation type, it must be the same type as this pass manager. void OpPassManager::addPass(std::unique_ptr pass) { - // If this pass runs on a different operation than this pass manager, then - // implicitly nest a pass manager for this operation. - auto passOpName = pass->getOpName(); - if (passOpName && passOpName != impl->name.getStringRef()) - return nest(*passOpName).addPass(std::move(pass)); - - impl->passes.emplace_back(std::move(pass)); - if (impl->verifyPasses) - impl->passes.emplace_back(std::make_unique()); + impl->addPass(std::move(pass)); } /// Returns the number of passes held by this manager. @@ -272,19 +325,17 @@ OpPassManagerImpl &OpPassManager::getImpl() { return *impl; } /// Return an instance of the context. -MLIRContext *OpPassManager::getContext() const { - return impl->name.getAbstractOperation()->dialect.getContext(); -} +MLIRContext *OpPassManager::getContext() const { return impl->getContext(); } /// Return the operation name that this pass manager operates on. const OperationName &OpPassManager::getOpName() const { return impl->name; } -/// Prints out the passes of the pass manager as the textual representation -/// of pipelines. -void OpPassManager::printAsTextualPipeline(raw_ostream &os) { +/// Prints out the given passes as the textual representation of a pipeline. +static void printAsTextualPipeline(ArrayRef> passes, + raw_ostream &os) { // Filter out passes that are not part of the public pipeline. - auto filteredPasses = llvm::make_filter_range( - impl->passes, [](const std::unique_ptr &pass) { + auto filteredPasses = + llvm::make_filter_range(passes, [](const std::unique_ptr &pass) { return !isa(pass); }); llvm::interleaveComma(filteredPasses, os, @@ -293,6 +344,12 @@ }); } +/// Prints out the passes of the pass manager as the textual representation +/// of pipelines. +void OpPassManager::printAsTextualPipeline(raw_ostream &os) { + ::printAsTextualPipeline(impl->passes, os); +} + //===----------------------------------------------------------------------===// // OpToOpPassAdaptor //===----------------------------------------------------------------------===// @@ -488,55 +545,75 @@ // PassCrashReproducer //===----------------------------------------------------------------------===// -/// Safely run the pass manager over the given module, creating a reproducible -/// on failure or crash. -static LogicalResult runWithCrashRecovery(OpPassManager &pm, - ModuleAnalysisManager &am, - ModuleOp module, - StringRef crashReproducerFileName) { +/// Run the pass manager with crash recover enabled. +LogicalResult PassManager::runWithCrashRecovery(ModuleOp module, + AnalysisManager am) { + // If this isn't a minimal producer, run all of the passes in recovery mode. + if (!minimalReproducer) + return runWithCrashRecovery(impl->passes, module, am); + + // Split the passes within adaptors to ensure that each pass can be run in + // isolation. + impl->splitAdaptorPasses(); + + // If this is a minimal producer, run each of the passes individually. If the + // verifier is enabled, each pass will have a verifier after. This is included + // in the recovery run. + unsigned stride = impl->verifyPasses ? 2 : 1; + MutableArrayRef> passes = impl->passes; + for (unsigned i = 0, e = passes.size(); i != e; i += stride) { + if (failed(runWithCrashRecovery(passes.slice(i, stride), module, am))) + return failure(); + } + return success(); +} + +/// Run the given passes with crash recover enabled. +LogicalResult +PassManager::runWithCrashRecovery(MutableArrayRef> passes, + ModuleOp module, AnalysisManager am) { /// Enable crash recovery. llvm::CrashRecoveryContext::Enable(); - // Grab the textual pipeline executing within the pass manager first, just in - // case the pass manager becomes compromised. + // Grab the textual pipeline being executed first, just in case the passes + // become compromised. std::string pipeline; { llvm::raw_string_ostream pipelineOS(pipeline); - pm.printAsTextualPipeline(pipelineOS); + ::printAsTextualPipeline(passes, pipelineOS); } // Clone the initial module before running it through the pass pipeline. OwningModuleRef reproducerModule = module.clone(); - // Safely invoke the pass manager within a recovery context. + // Safely invoke the passes within a recovery context. LogicalResult passManagerResult = failure(); llvm::CrashRecoveryContext recoveryContext; - recoveryContext.RunSafelyOnThread( - [&] { passManagerResult = pm.run(module, am); }); - - /// Disable crash recovery. + recoveryContext.RunSafelyOnThread([&] { + for (std::unique_ptr &pass : passes) + if (failed(pass->run(module, am))) + return; + passManagerResult = success(); + }); llvm::CrashRecoveryContext::Disable(); if (succeeded(passManagerResult)) return success(); - // The conversion failed, so generate a reproducible. std::string error; std::unique_ptr outputFile = - mlir::openOutputFile(crashReproducerFileName, &error); + mlir::openOutputFile(*crashReproducerFileName, &error); if (!outputFile) - return emitError(UnknownLoc::get(pm.getContext()), - ": ") - << error; + return module.emitError(": ") << error; auto &outputOS = outputFile->os(); // Output the current pass manager configuration. outputOS << "// configuration: -pass-pipeline='" << pipeline << "'"; - if (pm.getImpl().disableThreads) + if (impl->disableThreads) outputOS << " -disable-pass-threading"; - // TODO(riverriddle) Should this also be configured with a pass manager flag? + // TODO: Should this also be configured with a pass manager flag? outputOS << "\n// note: verifyPasses=" - << (pm.getImpl().verifyPasses ? "true" : "false") << "\n"; + << (impl->verifyPasses ? "true" : "false") << "\n"; // Output the .mlir module. reproducerModule->print(outputOS); @@ -545,7 +622,7 @@ return reproducerModule->emitError() << "A failure has been detected while processing the MLIR module, a " "reproducer has been generated in '" - << crashReproducerFileName << "'"; + << *crashReproducerFileName << "'"; } //===----------------------------------------------------------------------===// @@ -555,7 +632,7 @@ PassManager::PassManager(MLIRContext *ctx, bool verifyPasses) : OpPassManager(OperationName(ModuleOp::getOperationName(), ctx), /*disableThreads=*/false, verifyPasses), - passTiming(false) {} + passTiming(false), minimalReproducer(false) {} PassManager::~PassManager() {} @@ -570,10 +647,9 @@ // If reproducer generation is enabled, run the pass manager with crash // handling enabled. - LogicalResult result = - crashReproducerFileName - ? runWithCrashRecovery(*this, am, module, *crashReproducerFileName) - : OpPassManager::run(module, am); + LogicalResult result = crashReproducerFileName + ? runWithCrashRecovery(module, am) + : OpPassManager::run(module, am); // Dump all of the pass statistics if necessary. if (passStatisticsMode) @@ -592,9 +668,12 @@ /// Enable support for the pass manager to generate a reproducer on the event /// of a crash or a pass failure. `outputFile` is a .mlir filename used to write -/// the generated reproducer. -void PassManager::enableCrashReproducerGeneration(StringRef outputFile) { +/// the generated reproducer. If `genMinimalReproducer` is true, the pass +/// manager will attempt to generate a minimal reproducer. +void PassManager::enableCrashReproducerGeneration(StringRef outputFile, + bool genMinimalReproducer) { crashReproducerFileName = std::string(outputFile); + minimalReproducer = genMinimalReproducer; } /// Add the provided instrumentation to the pass manager. diff --git a/mlir/lib/Pass/PassManagerOptions.cpp b/mlir/lib/Pass/PassManagerOptions.cpp --- a/mlir/lib/Pass/PassManagerOptions.cpp +++ b/mlir/lib/Pass/PassManagerOptions.cpp @@ -23,6 +23,11 @@ "pass-pipeline-crash-reproducer", llvm::cl::desc("Generate a .mlir reproducer file at the given output path" " if the pass manager crashes or fails")}; + llvm::cl::opt minimalReproducer{ + "pass-pipeline-minimal-reproducer", + llvm::cl::desc("When generating a crash reproducer, attempt to generated " + "a reproducer with the smallest pipeline."), + llvm::cl::init(false)}; //===--------------------------------------------------------------------===// // Multi-threading @@ -155,7 +160,8 @@ // Generate a reproducer on crash/failure. if (options->reproducerFile.getNumOccurrences()) - pm.enableCrashReproducerGeneration(options->reproducerFile); + pm.enableCrashReproducerGeneration(options->reproducerFile, + options->minimalReproducer); // Disable multi-threading. if (options->disableThreads) diff --git a/mlir/test/Pass/crash-recovery.mlir b/mlir/test/Pass/crash-recovery.mlir --- a/mlir/test/Pass/crash-recovery.mlir +++ b/mlir/test/Pass/crash-recovery.mlir @@ -1,5 +1,7 @@ // RUN: mlir-opt %s -pass-pipeline='func(test-function-pass, test-pass-crash)' -pass-pipeline-crash-reproducer=%t -verify-diagnostics // RUN: cat %t | FileCheck -check-prefix=REPRO %s +// RUN: mlir-opt %s -pass-pipeline='func(test-function-pass, test-pass-crash)' -pass-pipeline-crash-reproducer=%t -verify-diagnostics -pass-pipeline-minimal-reproducer +// RUN: cat %t | FileCheck -check-prefix=REPRO_MINIMAL %s // expected-error@+1 {{A failure has been detected while processing the MLIR module}} module { @@ -13,3 +15,9 @@ // REPRO: module // REPRO: func @foo() { // REPRO-NEXT: return + +// REPRO_MINIMAL: configuration: -pass-pipeline='func(test-pass-crash)' + +// REPRO_MINIMAL: module +// REPRO_MINIMAL: func @foo() { +// REPRO_MINIMAL-NEXT: return