[llvm] r288023 - [PM] Remove weird marking of invalidated analyses as "preserved".

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 28 02:42:21 PST 2016


Author: chandlerc
Date: Mon Nov 28 04:42:21 2016
New Revision: 288023

URL: http://llvm.org/viewvc/llvm-project?rev=288023&view=rev
Log:
[PM] Remove weird marking of invalidated analyses as "preserved".

This never made a lot of sense. They've been invalidated for one IR unit
but they aren't really preserved in any normal sense. It seemed like it
would be an elegant way of communicating to outer IR units that pass
managers and adaptors had already handled invalidation, but we've since
ended up adding sets that model this more clearly: we're now using
the 'AllAnalysesOn<IRUnitT>' set to handle cases where the trick of
"preserving" invalidated analyses didn't work.

This patch moves to rely on that technique exclusively and removes the
cumbersome API aspect of updating the preserved set when doing
invalidation. This in turn will simplify a *number* of upcoming patches.

This has a side benefit of exposing a number of places where we were
failing to mark the 'AllAnalysesOn<IRUnitT>' set as preserved. This
patch fixes those, and with those fixes shouldn't change any observable
behavior.

Modified:
    llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h
    llvm/trunk/include/llvm/Analysis/LoopPassManager.h
    llvm/trunk/include/llvm/IR/PassManager.h
    llvm/trunk/lib/Analysis/CGSCCPassManager.cpp

Modified: llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h?rev=288023&r1=288022&r2=288023&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h (original)
+++ llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h Mon Nov 28 04:42:21 2016
@@ -345,8 +345,7 @@ public:
             // whatever was updating the call graph. This SCC gets invalidated
             // late as it contains the nodes that were actively being
             // processed.
-            PassPA = CGAM.invalidate(*(UR.UpdatedC ? UR.UpdatedC : C),
-                                     std::move(PassPA));
+            CGAM.invalidate(*(UR.UpdatedC ? UR.UpdatedC : C), PassPA);
 
             // Then intersect the preserved set so that invalidation of module
             // analyses will eventually occur when the module pass completes.
@@ -373,10 +372,11 @@ public:
       } while (!RCWorklist.empty());
     }
 
-    // By definition we preserve the proxy. This precludes *any* invalidation
-    // of CGSCC analyses by the proxy, but that's OK because we've taken
-    // care to invalidate analyses in the CGSCC analysis manager
-    // incrementally above.
+    // By definition we preserve the proxy. We also preserve all analyses on
+    // SCCs. This precludes *any* invalidation of CGSCC analyses by the proxy,
+    // but that's OK because we've taken care to invalidate analyses in the
+    // CGSCC analysis manager incrementally above.
+    PA.preserve<AllAnalysesOn<LazyCallGraph::SCC>>();
     PA.preserve<CGSCCAnalysisManagerModuleProxy>();
     return PA;
   }
@@ -479,9 +479,7 @@ public:
       // We know that the function pass couldn't have invalidated any other
       // function's analyses (that's the contract of a function pass), so
       // directly handle the function analysis manager's invalidation here.
-      // Also, update the preserved analyses to reflect that once invalidated
-      // these can again be preserved.
-      PassPA = FAM.invalidate(N->getFunction(), std::move(PassPA));
+      FAM.invalidate(N->getFunction(), PassPA);
 
       // Then intersect the preserved set so that invalidation of module
       // analyses will eventually occur when the module pass completes.
@@ -495,10 +493,11 @@ public:
              "Current SCC not updated to the SCC containing the current node!");
     }
 
-    // By definition we preserve the proxy. This precludes *any* invalidation
-    // of function analyses by the proxy, but that's OK because we've taken
-    // care to invalidate analyses in the function analysis manager
-    // incrementally above.
+    // By definition we preserve the proxy. And we preserve all analyses on
+    // Functions. This precludes *any* invalidation of function analyses by the
+    // proxy, but that's OK because we've taken care to invalidate analyses in
+    // the function analysis manager incrementally above.
+    PA.preserve<AllAnalysesOn<Function>>();
     PA.preserve<FunctionAnalysisManagerCGSCCProxy>();
 
     // We've also ensured that we updated the call graph along the way.

Modified: llvm/trunk/include/llvm/Analysis/LoopPassManager.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/LoopPassManager.h?rev=288023&r1=288022&r2=288023&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/LoopPassManager.h (original)
+++ llvm/trunk/include/llvm/Analysis/LoopPassManager.h Mon Nov 28 04:42:21 2016
@@ -94,19 +94,19 @@ public:
 
       // We know that the loop pass couldn't have invalidated any other loop's
       // analyses (that's the contract of a loop pass), so directly handle the
-      // loop analysis manager's invalidation here.  Also, update the
-      // preserved analyses to reflect that once invalidated these can again
-      // be preserved.
-      PassPA = LAM.invalidate(*L, std::move(PassPA));
+      // loop analysis manager's invalidation here.
+      LAM.invalidate(*L, PassPA);
 
       // Then intersect the preserved set so that invalidation of module
       // analyses will eventually occur when the module pass completes.
       PA.intersect(std::move(PassPA));
     }
 
-    // By definition we preserve the proxy. This precludes *any* invalidation of
-    // loop analyses by the proxy, but that's OK because we've taken care to
-    // invalidate analyses in the loop analysis manager incrementally above.
+    // By definition we preserve the proxy. We also preserve all analyses on
+    // Loops. This precludes *any* invalidation of loop analyses by the proxy,
+    // but that's OK because we've taken care to invalidate analyses in the
+    // loop analysis manager incrementally above.
+    PA.preserve<AllAnalysesOn<Loop>>();
     PA.preserve<LoopAnalysisManagerFunctionProxy>();
     return PA;
   }

Modified: llvm/trunk/include/llvm/IR/PassManager.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/PassManager.h?rev=288023&r1=288022&r2=288023&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/PassManager.h (original)
+++ llvm/trunk/include/llvm/IR/PassManager.h Mon Nov 28 04:42:21 2016
@@ -281,13 +281,11 @@ public:
       PreservedAnalyses PassPA = Passes[Idx]->run(IR, AM, ExtraArgs...);
 
       // Update the analysis manager as each pass runs and potentially
-      // invalidates analyses. We also update the preserved set of analyses
-      // based on what analyses we have already handled the invalidation for
-      // here and don't need to invalidate when finished.
-      PassPA = AM.invalidate(IR, std::move(PassPA));
+      // invalidates analyses.
+      AM.invalidate(IR, PassPA);
 
-      // Finally, we intersect the final preserved analyses to compute the
-      // aggregate preserved set for this pass manager.
+      // Finally, we intersect the preserved analyses to compute the aggregate
+      // preserved set for this pass manager.
       PA.intersect(std::move(PassPA));
 
       // FIXME: Historically, the pass managers all called the LLVM context's
@@ -476,13 +474,10 @@ public:
   ///
   /// Walk through all of the analyses pertaining to this unit of IR and
   /// invalidate them unless they are preserved by the PreservedAnalyses set.
-  /// We accept the PreservedAnalyses set by value and update it with each
-  /// analyis pass which has been successfully invalidated and thus can be
-  /// preserved going forward. The updated set is returned.
-  PreservedAnalyses invalidate(IRUnitT &IR, PreservedAnalyses PA) {
+  void invalidate(IRUnitT &IR, const PreservedAnalyses &PA) {
     // Short circuit for common cases of all analyses being preserved.
     if (PA.areAllPreserved() || PA.preserved<AllAnalysesOn<IRUnitT>>())
-      return PA;
+      return;
 
     if (DebugLogging)
       dbgs() << "Invalidating all non-preserved analyses for: " << IR.getName()
@@ -510,18 +505,13 @@ public:
       } else {
         ++I;
       }
-
-      // After handling each pass, we mark it as preserved. Once we've
-      // invalidated any stale results, the rest of the system is allowed to
-      // start preserving this analysis again.
-      PA.preserve(ID);
     }
     while (!InvalidatedIDs.empty())
       AnalysisResults.erase(std::make_pair(InvalidatedIDs.pop_back_val(), &IR));
     if (ResultsList.empty())
       AnalysisResultLists.erase(&IR);
 
-    return PA;
+    return;
   }
 
 private:
@@ -846,20 +836,19 @@ public:
 
       // We know that the function pass couldn't have invalidated any other
       // function's analyses (that's the contract of a function pass), so
-      // directly handle the function analysis manager's invalidation here and
-      // update our preserved set to reflect that these have already been
-      // handled.
-      PassPA = FAM.invalidate(F, std::move(PassPA));
+      // directly handle the function analysis manager's invalidation here.
+      FAM.invalidate(F, PassPA);
 
       // Then intersect the preserved set so that invalidation of module
       // analyses will eventually occur when the module pass completes.
       PA.intersect(std::move(PassPA));
     }
 
-    // By definition we preserve the proxy. This precludes *any* invalidation
-    // of function analyses by the proxy, but that's OK because we've taken
-    // care to invalidate analyses in the function analysis manager
-    // incrementally above.
+    // By definition we preserve the proxy. We also preserve all analyses on
+    // Function units. This precludes *any* invalidation of function analyses
+    // by the proxy, but that's OK because we've taken care to invalidate
+    // analyses in the function analysis manager incrementally above.
+    PA.preserve<AllAnalysesOn<Function>>();
     PA.preserve<FunctionAnalysisManagerModuleProxy>();
     return PA;
   }

Modified: llvm/trunk/lib/Analysis/CGSCCPassManager.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/CGSCCPassManager.cpp?rev=288023&r1=288022&r2=288023&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/CGSCCPassManager.cpp (original)
+++ llvm/trunk/lib/Analysis/CGSCCPassManager.cpp Mon Nov 28 04:42:21 2016
@@ -56,10 +56,8 @@ PassManager<LazyCallGraph::SCC, CGSCCAna
     assert(C->begin() != C->end() && "Cannot have an empty SCC!");
 
     // Update the analysis manager as each pass runs and potentially
-    // invalidates analyses. We also update the preserved set of analyses
-    // based on what analyses we have already handled the invalidation for
-    // here and don't need to invalidate when finished.
-    PassPA = AM.invalidate(*C, std::move(PassPA));
+    // invalidates analyses.
+    AM.invalidate(*C, PassPA);
 
     // Finally, we intersect the final preserved analyses to compute the
     // aggregate preserved set for this pass manager.
@@ -72,6 +70,12 @@ PassManager<LazyCallGraph::SCC, CGSCCAna
     // ...getContext().yield();
   }
 
+  // Invaliadtion was handled after each pass in the above loop for the current
+  // SCC. Therefore, the remaining analysis results in the AnalysisManager are
+  // preserved. We mark this with a set so that we don't need to inspect each
+  // one individually.
+  PA.preserve<AllAnalysesOn<LazyCallGraph::SCC>>();
+
   if (DebugLogging)
     dbgs() << "Finished CGSCC pass manager run.\n";
 




More information about the llvm-commits mailing list