[NewPM] fixing asserts on deleted loop in -print-after-all

IR-printing AfterPass instrumentation might be called on a loop
that has just been invalidated. We should skip printing it to
avoid spurious asserts.

Reviewed By: chandlerc, philip.pfaffe
Differential Revision: https://reviews.llvm.org/D54740

llvm-svn: 348887
This commit is contained in:
Fedor Sergeev 2018-12-11 19:05:35 +00:00
parent 8876dac50a
commit a1d95c3fc4
11 changed files with 233 additions and 17 deletions

View File

@ -441,7 +441,10 @@ public:
PreservedAnalyses PassPA = Pass.run(*C, CGAM, CG, UR);
PI.runAfterPass<LazyCallGraph::SCC>(Pass, *C);
if (UR.InvalidatedSCCs.count(C))
PI.runAfterPassInvalidated<LazyCallGraph::SCC>(Pass);
else
PI.runAfterPass<LazyCallGraph::SCC>(Pass, *C);
// Update the SCC and RefSCC if necessary.
C = UR.UpdatedC ? UR.UpdatedC : C;
@ -762,7 +765,10 @@ public:
PreservedAnalyses PassPA = Pass.run(*C, AM, CG, UR);
PI.runAfterPass<LazyCallGraph::SCC>(Pass, *C);
if (UR.InvalidatedSCCs.count(C))
PI.runAfterPassInvalidated<LazyCallGraph::SCC>(Pass);
else
PI.runAfterPass<LazyCallGraph::SCC>(Pass, *C);
// If the SCC structure has changed, bail immediately and let the outer
// CGSCC layer handle any iteration to reflect the refined structure.

View File

@ -68,10 +68,17 @@ class PreservedAnalyses;
/// PassInstrumentation to pass control to the registered callbacks.
class PassInstrumentationCallbacks {
public:
// Before/After callbacks accept IRUnits, so they need to take them
// as pointers, wrapped with llvm::Any
// Before/After callbacks accept IRUnits whenever appropriate, so they need
// to take them as constant pointers, wrapped with llvm::Any.
// For the case when IRUnit has been invalidated there is a different
// callback to use - AfterPassInvalidated.
// TODO: currently AfterPassInvalidated does not accept IRUnit, since passing
// already invalidated IRUnit is unsafe. There are ways to handle invalidated IRUnits
// in a safe way, and we might pursue that as soon as there is a useful instrumentation
// that needs it.
using BeforePassFunc = bool(StringRef, Any);
using AfterPassFunc = void(StringRef, Any);
using AfterPassInvalidatedFunc = void(StringRef);
using BeforeAnalysisFunc = void(StringRef, Any);
using AfterAnalysisFunc = void(StringRef, Any);
@ -90,6 +97,11 @@ public:
AfterPassCallbacks.emplace_back(std::move(C));
}
template <typename CallableT>
void registerAfterPassInvalidatedCallback(CallableT C) {
AfterPassInvalidatedCallbacks.emplace_back(std::move(C));
}
template <typename CallableT>
void registerBeforeAnalysisCallback(CallableT C) {
BeforeAnalysisCallbacks.emplace_back(std::move(C));
@ -105,6 +117,8 @@ private:
SmallVector<llvm::unique_function<BeforePassFunc>, 4> BeforePassCallbacks;
SmallVector<llvm::unique_function<AfterPassFunc>, 4> AfterPassCallbacks;
SmallVector<llvm::unique_function<AfterPassInvalidatedFunc>, 4>
AfterPassInvalidatedCallbacks;
SmallVector<llvm::unique_function<BeforeAnalysisFunc>, 4>
BeforeAnalysisCallbacks;
SmallVector<llvm::unique_function<AfterAnalysisFunc>, 4>
@ -139,7 +153,8 @@ public:
}
/// AfterPass instrumentation point - takes \p Pass instance that has
/// just been executed and constant reference to IR it operates on.
/// just been executed and constant reference to \p IR it operates on.
/// \p IR is guaranteed to be valid at this point.
template <typename IRUnitT, typename PassT>
void runAfterPass(const PassT &Pass, const IRUnitT &IR) const {
if (Callbacks)
@ -147,6 +162,16 @@ public:
C(Pass.name(), llvm::Any(&IR));
}
/// AfterPassInvalidated instrumentation point - takes \p Pass instance
/// that has just been executed. For use when IR has been invalidated
/// by \p Pass execution.
template <typename IRUnitT, typename PassT>
void runAfterPassInvalidated(const PassT &Pass) const {
if (Callbacks)
for (auto &C : Callbacks->AfterPassInvalidatedCallbacks)
C(Pass.name());
}
/// BeforeAnalysis instrumentation point - takes \p Analysis instance
/// to be executed and constant reference to IR it operates on.
template <typename IRUnitT, typename PassT>

View File

@ -99,8 +99,8 @@ private:
void stopTimer(StringRef PassID);
// Implementation of pass instrumentation callbacks.
bool runBeforePass(StringRef PassID, Any IR);
void runAfterPass(StringRef PassID, Any IR);
bool runBeforePass(StringRef PassID);
void runAfterPass(StringRef PassID);
};
} // namespace llvm

View File

@ -352,7 +352,11 @@ public:
continue;
PreservedAnalyses PassPA = Pass.run(*L, LAM, LAR, Updater);
PI.runAfterPass<Loop>(Pass, *L);
// Do not pass deleted Loop into the instrumentation.
if (Updater.skipCurrentLoop())
PI.runAfterPassInvalidated<Loop>(Pass);
else
PI.runAfterPass<Loop>(Pass, *L);
// FIXME: We should verify the set of analyses relevant to Loop passes
// are preserved.

View File

@ -79,7 +79,10 @@ PassManager<LazyCallGraph::SCC, CGSCCAnalysisManager, LazyCallGraph &,
PreservedAnalyses PassPA = Pass->run(*C, AM, G, UR);
PI.runAfterPass(*Pass, *C);
if (UR.InvalidatedSCCs.count(C))
PI.runAfterPassInvalidated<LazyCallGraph::SCC>(*Pass);
else
PI.runAfterPass<LazyCallGraph::SCC>(*Pass, *C);
// Update the SCC if necessary.
C = UR.UpdatedC ? UR.UpdatedC : C;

View File

@ -226,7 +226,7 @@ static bool matchPassManager(StringRef PassID) {
Prefix.endswith("AnalysisManagerProxy");
}
bool TimePassesHandler::runBeforePass(StringRef PassID, Any IR) {
bool TimePassesHandler::runBeforePass(StringRef PassID) {
if (matchPassManager(PassID))
return true;
@ -239,7 +239,7 @@ bool TimePassesHandler::runBeforePass(StringRef PassID, Any IR) {
return true;
}
void TimePassesHandler::runAfterPass(StringRef PassID, Any IR) {
void TimePassesHandler::runAfterPass(StringRef PassID) {
if (matchPassManager(PassID))
return;
@ -254,13 +254,15 @@ void TimePassesHandler::registerCallbacks(PassInstrumentationCallbacks &PIC) {
return;
PIC.registerBeforePassCallback(
[this](StringRef P, Any IR) { return this->runBeforePass(P, IR); });
[this](StringRef P, Any) { return this->runBeforePass(P); });
PIC.registerAfterPassCallback(
[this](StringRef P, Any IR) { this->runAfterPass(P, IR); });
[this](StringRef P, Any) { this->runAfterPass(P); });
PIC.registerAfterPassInvalidatedCallback(
[this](StringRef P) { this->runAfterPass(P); });
PIC.registerBeforeAnalysisCallback(
[this](StringRef P, Any IR) { this->runBeforePass(P, IR); });
[this](StringRef P, Any) { this->runBeforePass(P); });
PIC.registerAfterAnalysisCallback(
[this](StringRef P, Any IR) { this->runAfterPass(P, IR); });
[this](StringRef P, Any) { this->runAfterPass(P); });
}
} // namespace llvm

View File

@ -53,7 +53,6 @@ void unwrapAndPrint(StringRef Banner, Any IR) {
Extra = formatv(" (function: {0})\n", F->getName());
} else if (any_isa<const LazyCallGraph::SCC *>(IR)) {
const LazyCallGraph::SCC *C = any_cast<const LazyCallGraph::SCC *>(IR);
assert(C);
if (!llvm::forcePrintModuleIR()) {
Extra = formatv(" (scc: {0})\n", C->getName());
bool BannerPrinted = false;

View File

@ -44,7 +44,11 @@ PassManager<Loop, LoopAnalysisManager, LoopStandardAnalysisResults &,
PreservedAnalyses PassPA = Pass->run(L, AM, AR, U);
PI.runAfterPass<Loop>(*Pass, L);
// do not pass deleted Loop into the instrumentation
if (U.skipCurrentLoop())
PI.runAfterPassInvalidated<Loop>(*Pass);
else
PI.runAfterPass<Loop>(*Pass, L);
// If the loop was deleted, abort the run and return to the outer walk.
if (U.skipCurrentLoop()) {

View File

@ -0,0 +1,24 @@
; Make sure that Loop which was invalidated by loop-deletion
; does not lead to problems for -print-after-all and is just skipped.
;
; RUN: opt < %s -disable-output \
; RUN: -passes=loop-instsimplify -print-after-all 2>&1 | FileCheck %s -check-prefix=SIMPLIFY
; RUN: opt < %s -disable-output \
; RUN: -passes=loop-deletion,loop-instsimplify -print-after-all 2>&1 | FileCheck %s -check-prefix=DELETED
;
; SIMPLIFY: IR Dump {{.*}} LoopInstSimplifyPass
; DELETED-NOT: IR Dump {{.*}}LoopInstSimplifyPass
; DELETED-NOT: IR Dump {{.*}}LoopDeletionPass
define void @deleteme() {
entry:
br label %loop
loop:
%iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
%iv.next = add i32 %iv, 1
%check = icmp ult i32 %iv.next, 3
br i1 %check, label %loop, label %exit
exit:
ret void
}

View File

@ -0,0 +1,25 @@
; RUN: opt < %s 2>&1 -disable-output \
; RUN: -passes=inline -print-before-all -print-after-all | FileCheck %s -check-prefix=INL
; RUN: opt < %s 2>&1 -disable-output \
; RUN: -passes=inline -print-before-all -print-after-all -print-module-scope | FileCheck %s -check-prefix=INL-MOD
; INL: IR Dump Before {{InlinerPass .*scc: .tester, foo}}
; INL-NOT: IR Dump After {{InlinerPass}}
; INL: IR Dump Before {{InlinerPass .*scc: .tester}}
; INL: IR Dump After {{InlinerPass .*scc: .tester}}
; INL-MOD: IR Dump Before {{InlinerPass .*scc: .tester, foo}}
; INL-MOD-NOT: IR Dump After {{InlinerPass}}
; INL-MOD: IR Dump Before {{InlinerPass .*scc: .tester}}
; INL-MOD: IR Dump After {{InlinerPass .*scc: .tester}}
define void @tester() noinline {
call void @foo()
ret void
}
define internal void @foo() alwaysinline {
call void @tester()
ret void
}

View File

@ -167,6 +167,11 @@ struct MockPassHandle<Loop>
MOCK_METHOD4(run,
PreservedAnalyses(Loop &, LoopAnalysisManager &,
LoopStandardAnalysisResults &, LPMUpdater &));
static void invalidateLoop(Loop &L, LoopAnalysisManager &,
LoopStandardAnalysisResults &,
LPMUpdater &Updater) {
Updater.markLoopAsDeleted(L, L.getName());
}
MockPassHandle() { setDefaults(); }
};
@ -187,6 +192,11 @@ struct MockPassHandle<LazyCallGraph::SCC>
PreservedAnalyses(LazyCallGraph::SCC &, CGSCCAnalysisManager &,
LazyCallGraph &G, CGSCCUpdateResult &UR));
static void invalidateSCC(LazyCallGraph::SCC &C, CGSCCAnalysisManager &,
LazyCallGraph &, CGSCCUpdateResult &UR) {
UR.InvalidatedSCCs.insert(&C);
}
MockPassHandle() { setDefaults(); }
};
@ -310,6 +320,7 @@ struct MockPassInstrumentationCallbacks {
}
MOCK_METHOD2(runBeforePass, bool(StringRef PassID, llvm::Any));
MOCK_METHOD2(runAfterPass, void(StringRef PassID, llvm::Any));
MOCK_METHOD1(runAfterPassInvalidated, void(StringRef PassID));
MOCK_METHOD2(runBeforeAnalysis, void(StringRef PassID, llvm::Any));
MOCK_METHOD2(runAfterAnalysis, void(StringRef PassID, llvm::Any));
@ -319,6 +330,8 @@ struct MockPassInstrumentationCallbacks {
});
Callbacks.registerAfterPassCallback(
[this](StringRef P, llvm::Any IR) { this->runAfterPass(P, IR); });
Callbacks.registerAfterPassInvalidatedCallback(
[this](StringRef P) { this->runAfterPassInvalidated(P); });
Callbacks.registerBeforeAnalysisCallback([this](StringRef P, llvm::Any IR) {
return this->runBeforeAnalysis(P, IR);
});
@ -571,6 +584,11 @@ TEST_F(FunctionCallbacksTest, InstrumentedPasses) {
runAfterPass(HasNameRegex("MockPassHandle"), HasName("foo")))
.InSequence(PISequence);
// Our mock pass does not invalidate IR.
EXPECT_CALL(CallbacksHandle,
runAfterPassInvalidated(HasNameRegex("MockPassHandle")))
.Times(0);
StringRef PipelineText = "test-transform";
ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded())
<< "Pipeline was: " << PipelineText;
@ -595,6 +613,9 @@ TEST_F(FunctionCallbacksTest, InstrumentedSkippedPasses) {
// as well.
EXPECT_CALL(CallbacksHandle, runAfterPass(HasNameRegex("MockPassHandle"), _))
.Times(0);
EXPECT_CALL(CallbacksHandle,
runAfterPassInvalidated(HasNameRegex("MockPassHandle")))
.Times(0);
EXPECT_CALL(CallbacksHandle, runAfterPass(HasNameRegex("MockPassHandle"), _))
.Times(0);
EXPECT_CALL(CallbacksHandle,
@ -650,6 +671,55 @@ TEST_F(LoopCallbacksTest, InstrumentedPasses) {
runAfterPass(HasNameRegex("MockPassHandle"), HasName("loop")))
.InSequence(PISequence);
// Our mock pass does not invalidate IR.
EXPECT_CALL(CallbacksHandle,
runAfterPassInvalidated(HasNameRegex("MockPassHandle")))
.Times(0);
StringRef PipelineText = "test-transform";
ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded())
<< "Pipeline was: " << PipelineText;
PM.run(*M, AM);
}
TEST_F(LoopCallbacksTest, InstrumentedInvalidatingPasses) {
CallbacksHandle.registerPassInstrumentation();
// Non-mock instrumentation not specifically mentioned below can be ignored.
CallbacksHandle.ignoreNonMockPassInstrumentation("<string>");
CallbacksHandle.ignoreNonMockPassInstrumentation("foo");
CallbacksHandle.ignoreNonMockPassInstrumentation("loop");
EXPECT_CALL(AnalysisHandle, run(HasName("loop"), _, _));
EXPECT_CALL(PassHandle, run(HasName("loop"), _, _, _))
.WillOnce(DoAll(WithArgs<0, 1, 2, 3>(Invoke(PassHandle.invalidateLoop)),
WithArgs<0, 1, 2>(Invoke(getAnalysisResult))));
// PassInstrumentation calls should happen in-sequence, in the same order
// as passes/analyses are scheduled.
::testing::Sequence PISequence;
EXPECT_CALL(CallbacksHandle,
runBeforePass(HasNameRegex("MockPassHandle"), HasName("loop")))
.InSequence(PISequence);
EXPECT_CALL(
CallbacksHandle,
runBeforeAnalysis(HasNameRegex("MockAnalysisHandle"), HasName("loop")))
.InSequence(PISequence);
EXPECT_CALL(
CallbacksHandle,
runAfterAnalysis(HasNameRegex("MockAnalysisHandle"), HasName("loop")))
.InSequence(PISequence);
EXPECT_CALL(CallbacksHandle,
runAfterPassInvalidated(HasNameRegex("MockPassHandle")))
.InSequence(PISequence);
EXPECT_CALL(CallbacksHandle,
runAfterPassInvalidated(HasNameRegex("^PassManager")))
.InSequence(PISequence);
// Our mock pass invalidates IR, thus normal runAfterPass is never called.
EXPECT_CALL(CallbacksHandle,
runAfterPass(HasNameRegex("MockPassHandle"), HasName("loop")))
.Times(0);
StringRef PipelineText = "test-transform";
ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded())
<< "Pipeline was: " << PipelineText;
@ -675,6 +745,9 @@ TEST_F(LoopCallbacksTest, InstrumentedSkippedPasses) {
// as well.
EXPECT_CALL(CallbacksHandle, runAfterPass(HasNameRegex("MockPassHandle"), _))
.Times(0);
EXPECT_CALL(CallbacksHandle,
runAfterPassInvalidated(HasNameRegex("MockPassHandle")))
.Times(0);
EXPECT_CALL(CallbacksHandle,
runBeforeAnalysis(HasNameRegex("MockAnalysisHandle"), _))
.Times(0);
@ -727,6 +800,54 @@ TEST_F(CGSCCCallbacksTest, InstrumentedPasses) {
runAfterPass(HasNameRegex("MockPassHandle"), HasName("(foo)")))
.InSequence(PISequence);
// Our mock pass does not invalidate IR.
EXPECT_CALL(CallbacksHandle,
runAfterPassInvalidated(HasNameRegex("MockPassHandle")))
.Times(0);
StringRef PipelineText = "test-transform";
ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded())
<< "Pipeline was: " << PipelineText;
PM.run(*M, AM);
}
TEST_F(CGSCCCallbacksTest, InstrumentedInvalidatingPasses) {
CallbacksHandle.registerPassInstrumentation();
// Non-mock instrumentation not specifically mentioned below can be ignored.
CallbacksHandle.ignoreNonMockPassInstrumentation("<string>");
CallbacksHandle.ignoreNonMockPassInstrumentation("(foo)");
EXPECT_CALL(AnalysisHandle, run(HasName("(foo)"), _, _));
EXPECT_CALL(PassHandle, run(HasName("(foo)"), _, _, _))
.WillOnce(DoAll(WithArgs<0, 1, 2, 3>(Invoke(PassHandle.invalidateSCC)),
WithArgs<0, 1, 2>(Invoke(getAnalysisResult))));
// PassInstrumentation calls should happen in-sequence, in the same order
// as passes/analyses are scheduled.
::testing::Sequence PISequence;
EXPECT_CALL(CallbacksHandle,
runBeforePass(HasNameRegex("MockPassHandle"), HasName("(foo)")))
.InSequence(PISequence);
EXPECT_CALL(
CallbacksHandle,
runBeforeAnalysis(HasNameRegex("MockAnalysisHandle"), HasName("(foo)")))
.InSequence(PISequence);
EXPECT_CALL(
CallbacksHandle,
runAfterAnalysis(HasNameRegex("MockAnalysisHandle"), HasName("(foo)")))
.InSequence(PISequence);
EXPECT_CALL(CallbacksHandle,
runAfterPassInvalidated(HasNameRegex("MockPassHandle")))
.InSequence(PISequence);
EXPECT_CALL(CallbacksHandle,
runAfterPassInvalidated(HasNameRegex("^PassManager")))
.InSequence(PISequence);
// Our mock pass does invalidate IR, thus normal runAfterPass is never called.
EXPECT_CALL(CallbacksHandle,
runAfterPass(HasNameRegex("MockPassHandle"), HasName("(foo)")))
.Times(0);
StringRef PipelineText = "test-transform";
ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded())
<< "Pipeline was: " << PipelineText;
@ -752,6 +873,9 @@ TEST_F(CGSCCCallbacksTest, InstrumentedSkippedPasses) {
// as well.
EXPECT_CALL(CallbacksHandle, runAfterPass(HasNameRegex("MockPassHandle"), _))
.Times(0);
EXPECT_CALL(CallbacksHandle,
runAfterPassInvalidated(HasNameRegex("MockPassHandle")))
.Times(0);
EXPECT_CALL(CallbacksHandle,
runBeforeAnalysis(HasNameRegex("MockAnalysisHandle"), _))
.Times(0);