Skip to content

Commit 87de8f3

Browse files
zyx-billySterling-Augustine
authored andcommitted
[MLIR][Pass] Full & deterministic diagnostics (llvm#110311)
Today, when the pass infra schedules a pass/nested-pipeline on a set of ops, it exits early as soon as it fails on one of the ops. This leads to non-exhaustive, and more importantly, non-deterministic error reporting (under async). This PR removes the early termination behavior so that all ops have a chance to run through the current pass/nested-pipeline, and all errors are reported (async diagnostics are already ordered). This guarantees deterministic & full error reporting. As a result, it's also no longer necessary to -split-input-file with one error per split when testing with -verify-diagnostics.
1 parent cc2fc35 commit 87de8f3

File tree

4 files changed

+36
-7
lines changed

4 files changed

+36
-7
lines changed

mlir/lib/Pass/Pass.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -732,7 +732,7 @@ void OpToOpPassAdaptor::runOnOperationImpl(bool verifyPasses) {
732732
unsigned initGeneration = mgr->impl->initializationGeneration;
733733
if (failed(runPipeline(*mgr, &op, am.nest(&op), verifyPasses,
734734
initGeneration, instrumentor, &parentInfo)))
735-
return signalPassFailure();
735+
signalPassFailure();
736736
}
737737
}
738738
}
@@ -799,7 +799,8 @@ void OpToOpPassAdaptor::runOnOperationAsyncImpl(bool verifyPasses) {
799799
// An atomic failure variable for the async executors.
800800
std::vector<std::atomic<bool>> activePMs(asyncExecutors.size());
801801
std::fill(activePMs.begin(), activePMs.end(), false);
802-
auto processFn = [&](OpPMInfo &opInfo) {
802+
std::atomic<bool> hasFailure = false;
803+
parallelForEach(context, opInfos, [&](OpPMInfo &opInfo) {
803804
// Find an executor for this operation.
804805
auto it = llvm::find_if(activePMs, [](std::atomic<bool> &isActive) {
805806
bool expectedInactive = false;
@@ -812,14 +813,15 @@ void OpToOpPassAdaptor::runOnOperationAsyncImpl(bool verifyPasses) {
812813
LogicalResult pipelineResult = runPipeline(
813814
pm, opInfo.op, opInfo.am, verifyPasses,
814815
pm.impl->initializationGeneration, instrumentor, &parentInfo);
816+
if (failed(pipelineResult))
817+
hasFailure.store(true);
815818

816819
// Reset the active bit for this pass manager.
817820
activePMs[pmIndex].store(false);
818-
return pipelineResult;
819-
};
821+
});
820822

821823
// Signal a failure if any of the executors failed.
822-
if (failed(failableParallelForEach(context, opInfos, processFn)))
824+
if (hasFailure)
823825
signalPassFailure();
824826
}
825827

mlir/test/Pass/crash-recovery-dynamic-failure.mlir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,4 @@ module @inner_mod1 {
1515
// REPRO_LOCAL_DYNAMIC_FAILURE: module @inner_mod1
1616
// REPRO_LOCAL_DYNAMIC_FAILURE: module @foo {
1717

18-
// REPRO_LOCAL_DYNAMIC_FAILURE: pipeline: "builtin.module(builtin.module(test-pass-failure))"
18+
// REPRO_LOCAL_DYNAMIC_FAILURE: pipeline: "builtin.module(builtin.module(test-pass-failure{gen-diagnostics=false}))"

mlir/test/Pass/full_diagnostics.mlir

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(test-pass-failure{gen-diagnostics}))' -verify-diagnostics
2+
3+
// Test that all errors are reported.
4+
// expected-error@below {{illegal operation}}
5+
func.func @TestAlwaysIllegalOperationPass1() {
6+
return
7+
}
8+
9+
// expected-error@below {{illegal operation}}
10+
func.func @TestAlwaysIllegalOperationPass2() {
11+
return
12+
}
13+
14+
// expected-error@below {{illegal operation}}
15+
func.func @TestAlwaysIllegalOperationPass3() {
16+
return
17+
}

mlir/test/lib/Pass/TestPassManager.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,21 @@ struct TestCrashRecoveryPass
151151
struct TestFailurePass : public PassWrapper<TestFailurePass, OperationPass<>> {
152152
MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(TestFailurePass)
153153

154-
void runOnOperation() final { signalPassFailure(); }
154+
TestFailurePass() = default;
155+
TestFailurePass(const TestFailurePass &other) : PassWrapper(other) {}
156+
157+
void runOnOperation() final {
158+
signalPassFailure();
159+
if (genDiagnostics)
160+
mlir::emitError(getOperation()->getLoc(), "illegal operation");
161+
}
155162
StringRef getArgument() const final { return "test-pass-failure"; }
156163
StringRef getDescription() const final {
157164
return "Test a pass in the pass manager that always fails";
158165
}
166+
167+
Option<bool> genDiagnostics{*this, "gen-diagnostics",
168+
llvm::cl::desc("Generate a diagnostic message")};
159169
};
160170

161171
/// A test pass that creates an invalid operation in a function body.

0 commit comments

Comments
 (0)