Skip to content

Commit f093a89

Browse files
committed
[analysis] Add a new form of verification SILAnalysis::verifyFull().
This is a verification routine that is only invoked in PassManager destructors. I am going to use this to ensure that the PassManagerVerifierAnalysis only runs at such points (even when sil-verify-all is enabled) since it is too expensive to run otherwise. NOTE: The default implementation of verifyFull in this commit is a no-op. I wanted to have verify() be the default implementation of verifyFull(), but I do not have more time to invest in this and it seems to catch /real/ bugs, albeit bugs unrelated to pass manager notification verification. Instead I am going to file an SR for someone to look at it since I need to move on from this work back to semantic SIL. At least we will not have notification failure issues anymore and thus a large correctness issue in the compiler has been fixed. Forward progress! rdar://42301529
1 parent fc41370 commit f093a89

File tree

2 files changed

+28
-0
lines changed

2 files changed

+28
-0
lines changed

include/swift/SILOptimizer/Analysis/Analysis.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,18 @@ class SILAnalysis : public DeleteNotificationHandler {
137137
/// specific verification will do so.
138138
virtual void verify(SILFunction *F) const { verify(); }
139139

140+
/// Perform a potentially more expensive verification of the state of this
141+
/// analysis.
142+
///
143+
/// The purpose of this is to allow for more expensive verification that is
144+
/// fast enough to run at the end of a pass manager once vs in between all
145+
/// passes when -sil-verify-all is enabled.
146+
///
147+
/// TODO: By default this is a no-op, but really it should call
148+
/// verify(). Today doing a full verification seems to catch verification
149+
/// errors when compiling the stdlib/overlays.
150+
virtual void verifyFull() const {}
151+
140152
/// Verify that the function \p F can be used by the analysis.
141153
static void verifyFunction(SILFunction *F);
142154
};

lib/SILOptimizer/PassManager/PassManager.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,22 @@ void SILPassManager::execute() {
568568
SILPassManager::~SILPassManager() {
569569
assert(IRGenPasses.empty() && "Must add IRGen SIL passes that were "
570570
"registered to the list of transformations");
571+
// Before we do anything further, verify the module and our analyses. These
572+
// are natural points with which to verify.
573+
//
574+
// TODO: We currently do not verify the module here since the verifier asserts
575+
// in the normal build. This should be enabled and those problems resolved
576+
// either by changing the verifier or treating those asserts as signs of a
577+
// bug.
578+
for (auto *A : Analyses) {
579+
// We use verify full instead of just verify to ensure that passes that want
580+
// to run more expensive verification after a pass manager is destroyed
581+
// properly trigger.
582+
//
583+
// NOTE: verifyFull() has a default implementation that just calls
584+
// verify(). So functionally, there is no difference here.
585+
A->verifyFull();
586+
}
571587

572588
// Remove our deserialization notification handler.
573589
Mod->removeDeserializationNotificationHandler(

0 commit comments

Comments
 (0)