[Mlir-commits] [mlir] 983382f - [mlir][Pass] Add support for generating local crash reproducers
River Riddle
llvmlistbot at llvm.org
Wed Apr 29 15:23:25 PDT 2020
Author: River Riddle
Date: 2020-04-29T15:23:10-07:00
New Revision: 983382f134a83f51b4614ab4544ed093bcb52f5e
URL: https://github.com/llvm/llvm-project/commit/983382f134a83f51b4614ab4544ed093bcb52f5e
DIFF: https://github.com/llvm/llvm-project/commit/983382f134a83f51b4614ab4544ed093bcb52f5e.diff
LOG: [mlir][Pass] Add support for generating local crash reproducers
This revision adds a mode to the crash reproducer generator to attempt to generate a more local reproducer. This will attempt to generate a reproducer right before the offending pass that fails. This is useful for the majority of failures that are specific to a single pass, and situations where some passes in the pipeline are not registered with a specific tool.
Differential Revision: https://reviews.llvm.org/D78314
Added:
Modified:
mlir/docs/PassManagement.md
mlir/include/mlir/Pass/Pass.h
mlir/include/mlir/Pass/PassManager.h
mlir/lib/Pass/Pass.cpp
mlir/lib/Pass/PassManagerOptions.cpp
mlir/test/Pass/crash-recovery.mlir
Removed:
################################################################################
diff --git a/mlir/docs/PassManagement.md b/mlir/docs/PassManagement.md
index 8f1795fad211..90d30deed914 100644
--- a/mlir/docs/PassManagement.md
+++ b/mlir/docs/PassManagement.md
@@ -992,3 +992,28 @@ module {
}
}
```
+
+### Local Reproducer Generation
+
+An additional flag may be passed to
+`PassManager::enableCrashReproducerGeneration`, and specified via
+`pass-pipeline-local-reproducer` on the command line, that signals that the pass
+manager should attempt to generate a "local" 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
index 5e0098458755..7c0f9bd958a1 100644
--- a/mlir/include/mlir/Pass/Pass.h
+++ b/mlir/include/mlir/Pass/Pass.h
@@ -252,6 +252,9 @@ class Pass {
/// 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
index 98101b82f542..15c88128102e 100644
--- a/mlir/include/mlir/Pass/PassManager.h
+++ b/mlir/include/mlir/Pass/PassManager.h
@@ -106,6 +106,9 @@ class OpPassManager {
/// Allow access to the constructor.
friend class PassManager;
+
+ /// Allow access.
+ friend detail::OpPassManagerImpl;
};
//===----------------------------------------------------------------------===//
@@ -145,8 +148,11 @@ class PassManager : public OpPassManager {
/// 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 `genLocalReproducer` is true, the pass
+ /// manager will attempt to generate a local reproducer that contains the
+ /// smallest pipeline.
+ void enableCrashReproducerGeneration(StringRef outputFile,
+ bool genLocalReproducer = false);
//===--------------------------------------------------------------------===//
// Instrumentations
@@ -271,8 +277,12 @@ class PassManager : public OpPassManager {
/// 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<std::unique_ptr<Pass>> passes,
+ ModuleOp module, AnalysisManager am);
/// Flag that specifies if pass statistics should be dumped.
Optional<PassDisplayMode> passStatisticsMode;
@@ -282,6 +292,12 @@ class PassManager : public OpPassManager {
/// An optional filename to use when generating a crash reproducer if valid.
Optional<std::string> crashReproducerFileName;
+
+ /// Flag that specifies if pass timing is enabled.
+ bool passTiming : 1;
+
+ /// Flag that specifies if the generated crash reproducer should be local.
+ bool localReproducer : 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
index b6bef48cb3ec..8f08b8554e95 100644
--- a/mlir/lib/Pass/Pass.cpp
+++ b/mlir/lib/Pass/Pass.cpp
@@ -121,17 +121,32 @@ struct OpPassManagerImpl {
}
/// 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> 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 @@ struct OpPassManagerImpl {
} // 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<Pass>(adaptor));
+ return adaptor->getPassManagers().front();
+}
+
+void OpPassManagerImpl::addPass(std::unique_ptr<Pass> pass) {
+ // If this pass runs on a
diff erent 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<VerifierPass>());
+}
+
void OpPassManagerImpl::coalesceAdjacentAdaptorPasses() {
// Bail out early if there are no adaptor passes.
if (llvm::none_of(passes, [](std::unique_ptr<Pass> &pass) {
@@ -199,6 +238,31 @@ void OpPassManagerImpl::coalesceAdjacentAdaptorPasses() {
llvm::erase_if(passes, std::logical_not<std::unique_ptr<Pass>>());
}
+void OpPassManagerImpl::splitAdaptorPasses() {
+ std::vector<std::unique_ptr<Pass>> oldPasses;
+ std::swap(passes, oldPasses);
+
+ for (std::unique_ptr<Pass> &pass : oldPasses) {
+ // If this pass isn't an adaptor, move it directly to the new pass list.
+ auto *currentAdaptor = dyn_cast<OpToOpPassAdaptor>(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<Pass> &nestedPass : adaptorPM.getImpl().passes) {
+ if (!isa<VerifierPass>(nestedPass.get()))
+ nest(adaptorPM.getOpName()).addPass(std::move(nestedPass));
+ }
+ }
+ }
+}
+
//===----------------------------------------------------------------------===//
// OpPassManager
//===----------------------------------------------------------------------===//
@@ -242,27 +306,16 @@ LogicalResult OpPassManager::run(Operation *op, AnalysisManager am) {
/// 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<Pass>(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> pass) {
- // If this pass runs on a
diff erent 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<VerifierPass>());
+ impl->addPass(std::move(pass));
}
/// Returns the number of passes held by this manager.
@@ -272,19 +325,17 @@ size_t OpPassManager::size() const { return impl->passes.size(); }
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<std::unique_ptr<Pass>> 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> &pass) {
+ auto filteredPasses =
+ llvm::make_filter_range(passes, [](const std::unique_ptr<Pass> &pass) {
return !isa<VerifierPass>(pass);
});
llvm::interleaveComma(filteredPasses, os,
@@ -293,6 +344,12 @@ void OpPassManager::printAsTextualPipeline(raw_ostream &os) {
});
}
+/// 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 @@ void OpToOpPassAdaptor::runOnOperationAsyncImpl() {
// 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 local producer, run all of the passes in recovery mode.
+ if (!localReproducer)
+ 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 local 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<std::unique_ptr<Pass>> 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<std::unique_ptr<Pass>> 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> &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<llvm::ToolOutputFile> outputFile =
- mlir::openOutputFile(crashReproducerFileName, &error);
+ mlir::openOutputFile(*crashReproducerFileName, &error);
if (!outputFile)
- return emitError(UnknownLoc::get(pm.getContext()),
- "<MLIR-PassManager-Crash-Reproducer>: ")
- << error;
+ return module.emitError("<MLIR-PassManager-Crash-Reproducer>: ") << 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 @@ static LogicalResult runWithCrashRecovery(OpPassManager &pm,
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 @@ static LogicalResult runWithCrashRecovery(OpPassManager &pm,
PassManager::PassManager(MLIRContext *ctx, bool verifyPasses)
: OpPassManager(OperationName(ModuleOp::getOperationName(), ctx),
/*disableThreads=*/false, verifyPasses),
- passTiming(false) {}
+ passTiming(false), localReproducer(false) {}
PassManager::~PassManager() {}
@@ -570,10 +647,9 @@ LogicalResult PassManager::run(ModuleOp module) {
// 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,13 @@ bool PassManager::isMultithreadingEnabled() {
/// 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 `genLocalReproducer` is true, the pass manager
+/// will attempt to generate a local reproducer that contains the smallest
+/// pipeline.
+void PassManager::enableCrashReproducerGeneration(StringRef outputFile,
+ bool genLocalReproducer) {
crashReproducerFileName = std::string(outputFile);
+ localReproducer = genLocalReproducer;
}
/// Add the provided instrumentation to the pass manager.
diff --git a/mlir/lib/Pass/PassManagerOptions.cpp b/mlir/lib/Pass/PassManagerOptions.cpp
index 953faa28c28a..c1b1eee07ea5 100644
--- a/mlir/lib/Pass/PassManagerOptions.cpp
+++ b/mlir/lib/Pass/PassManagerOptions.cpp
@@ -23,6 +23,11 @@ struct PassManagerOptions {
"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<bool> localReproducer{
+ "pass-pipeline-local-reproducer",
+ llvm::cl::desc("When generating a crash reproducer, attempt to generated "
+ "a reproducer with the smallest pipeline."),
+ llvm::cl::init(false)};
//===--------------------------------------------------------------------===//
// Multi-threading
@@ -156,7 +161,8 @@ void mlir::applyPassManagerCLOptions(PassManager &pm) {
// Generate a reproducer on crash/failure.
if (options->reproducerFile.getNumOccurrences())
- pm.enableCrashReproducerGeneration(options->reproducerFile);
+ pm.enableCrashReproducerGeneration(options->reproducerFile,
+ options->localReproducer);
// Disable multi-threading.
if (options->disableThreads)
diff --git a/mlir/test/Pass/crash-recovery.mlir b/mlir/test/Pass/crash-recovery.mlir
index 5a2e88b48e88..2de7e505b445 100644
--- 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-local-reproducer
+// RUN: cat %t | FileCheck -check-prefix=REPRO_LOCAL %s
// expected-error at +1 {{A failure has been detected while processing the MLIR module}}
module {
@@ -13,3 +15,9 @@ module {
// REPRO: module
// REPRO: func @foo() {
// REPRO-NEXT: return
+
+// REPRO_LOCAL: configuration: -pass-pipeline='func(test-pass-crash)'
+
+// REPRO_LOCAL: module
+// REPRO_LOCAL: func @foo() {
+// REPRO_LOCAL-NEXT: return
More information about the Mlir-commits
mailing list