[PATCH] D60914: [AliasAnalysis] AAResults preserves AAManager.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

This looks right to me. Some suggestions on comment wording only.



================
Comment at: lib/Analysis/AliasAnalysis.cpp:82-83
                            FunctionAnalysisManager::Invalidator &Inv) {
-  // Check if the AA manager itself has been invalidated.
-  auto PAC = PA.getChecker<AAManager>();
-  if (!PAC.preserved() && !PAC.preservedSet<AllAnalysesOn<Function>>())
-    return true; // The manager needs to be blown away, clear everything.
+  // AAResults preserves the AAManager by default, due to the stateless nature
+  // of AliasAnalysis. Do not invalidate it here.
 
----------------
One nit about wording "Do not invalidate it here." -> "No need to check whether it has been preserved explicitly."

You should also update the comments on `AAManager` to explain this, and further add the (largely already in place) restriction that the registration functions must not be called once the `AAManager` is run.


I would also, merge this into the comment below as a single bit of prose. Could just continue after this comment with: "However, we still need to check if any of the dependencies end up being invalidated, and invalidate ourselves in that case."


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60914/new/

https://reviews.llvm.org/D60914