[PM] Fix proxy invalidation

[philip.pfaffe]

Summary

I made a mistake in handling transitive invalidation of analysis results. I've updated the list of preserved analyses as well as the correct result dependences.

[grosser]

Hi Philip,

could you explain what the mistake was and how it is resolved. The commit messages are an important way to educate people. It would be great if you could add some of your findings.

Best,
Tobias

[philip.pfaffe]

The Invalidator passed through the invalidate() path can be used to transitively invalidate analyses. It frequently happens that analysis results depend on other analyses, and thus store references to their results. When the dependee now gets invalidated, the depender needs to be invalidated as well. This is the purpose of the Invalidator object, which can be used to check whether some dependee analysis is in the process of being invalidated. I originally was checking the wrong dependee analyses, which is an actual error, you can only check analysis results that are in the cache (which they are if you've captured their reference). The invalidation I'm handling inside the proxy deals with the standard analyses the proxy passes into the Scop pipeline, since I'm capturing their reference.

This checking allows us to actually preserve a couple of results outside of the proxy, since the Scop pipeline shouldn't break those, or otherwise should update them accordingly (though this requires some further discussion).

[grosser]

LGTM. Can you add this to the commit message?

polly/trunk/include/polly/ScopPass.h

 
 private:
   bool runOnRegion(Region *R, RGPassManager &RGM) override;
   void print(raw_ostream &OS, const Module *) const override;
 };
 
 struct ScopStandardAnalysisResults {
   DominatorTree &DT;
+  ScopInfo &SI;
   ScalarEvolution &SE;
   LoopInfo &LI;
   RegionInfo &RI;
 };
 
 class SPMUpdater {
 public:
   SPMUpdater(SmallPriorityWorklist<Scop *, 4> &Worklist,
public:
   explicit FunctionToScopPassAdaptor(ScopPassT Pass) : Pass(std::move(Pass)) {}
 
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM) {
     PreservedAnalyses PA = PreservedAnalyses::all();
     auto &Scops = AM.getResult<ScopInfoAnalysis>(F);
     if (Scops.empty())
       return PA;
 
-    ScopAnalysisManager &SAM =
-        AM.getResult<ScopAnalysisManagerFunctionProxy>(F).getManager();
-
     ScopStandardAnalysisResults AR = {AM.getResult<DominatorTreeAnalysis>(F),
+                                      AM.getResult<ScopInfoAnalysis>(F),
                                       AM.getResult<ScalarEvolutionAnalysis>(F),
                                       AM.getResult<LoopAnalysis>(F),
                                       AM.getResult<RegionInfoAnalysis>(F)};
 
+    ScopAnalysisManager &SAM =
+        AM.getResult<ScopAnalysisManagerFunctionProxy>(F).getManager();
+
     SmallPriorityWorklist<Scop *, 4> Worklist;
     SPMUpdater Updater{Worklist, SAM};
 
     for (auto &S : Scops)
       if (auto *scop = S.second.get())
         Worklist.insert(scop);
 
     while (!Worklist.empty()) {
       Scop *scop = Worklist.pop_back_val();
       PreservedAnalyses PassPA = Pass.run(*scop, SAM, AR, Updater);
 
       SAM.invalidate(*scop, PassPA);
       PA.intersect(std::move(PassPA));
     };
 
     PA.preserveSet<AllAnalysesOn<Scop>>();
     PA.preserve<ScopAnalysisManagerFunctionProxy>();
+    PA.preserve<DominatorTreeAnalysis>();
+    PA.preserve<ScopAnalysis>();
+    PA.preserve<ScopInfoAnalysis>();
+    PA.preserve<ScalarEvolutionAnalysis>();
+    PA.preserve<LoopAnalysis>();
+    PA.preserve<RegionInfoAnalysis>();
     return PA;
   }
 
 private:
   ScopPassT Pass;
 }; // namespace polly
 
 template <typename ScopPassT>
 FunctionToScopPassAdaptor<ScopPassT>
 createFunctionToScopPassAdaptor(ScopPassT Pass) {
   return FunctionToScopPassAdaptor<ScopPassT>(std::move(Pass));
 }
 
 } // namespace polly
 
 #endif

polly/trunk/lib/Analysis/ScopPass.cpp

 
 bool ScopAnalysisManagerFunctionProxy::Result::invalidate(
     Function &F, const PreservedAnalyses &PA,
     FunctionAnalysisManager::Invalidator &Inv) {
 
   // First, check whether our ScopInfo is about to be invalidated
   auto PAC = PA.getChecker<ScopAnalysisManagerFunctionProxy>();
   if (!(PAC.preserved() || PAC.preservedSet<AllAnalysesOn<Function>>() ||
-        Inv.invalidate<ScopAnalysis>(F, PA) ||
+        Inv.invalidate<ScopInfoAnalysis>(F, PA) ||
         Inv.invalidate<ScalarEvolutionAnalysis>(F, PA) ||
         Inv.invalidate<LoopAnalysis>(F, PA) ||
-        Inv.invalidate<AAManager>(F, PA) ||
-        Inv.invalidate<DominatorTreeAnalysis>(F, PA) ||
-        Inv.invalidate<AssumptionAnalysis>(F, PA))) {
+        Inv.invalidate<DominatorTreeAnalysis>(F, PA))) {
 
     // As everything depends on ScopInfo, we must drop all existing results
     for (auto &S : *SI)
       if (auto *scop = S.second.get())
         if (InnerAM)
           InnerAM->clear(*scop);
 
     InnerAM = nullptr;