[PATCH] D61284: [AliasAnalysis/NewPassManager] Invalidate AAManager less often.

Chandler Carruth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 29 17:31:31 PDT 2019


chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

LGTM, very nice! Thanks for all the work tracking down this subtle case!

I've left some tiny suggestions below, but feel free to submit.



================
Comment at: include/llvm/Analysis/AliasAnalysis.h:1104
+/// longer be registered once the `AAManager` is run.
+
 class AAManager : public AnalysisInfoMixin<AAManager> {
----------------
stray blank line?


================
Comment at: include/llvm/IR/PassManager.h:289-291
+    // Return true if the checker's analysis was not abandoned, i.e. it was not
+    // explicitly invalidated. Even if the analysis is not explicitly
+    // preserved, if the analysis is known stateless, then it is preserved.
----------------
Make a doxygen comment?


================
Comment at: lib/Analysis/AliasAnalysis.cpp:86-89
+  // AAResults preserves the AAManager by default, due to the stateless nature
+  // of AliasAnalysis. There is no need to check whether it has been preserved
+  // explicitly. However, we still need to check if any of the dependencies end
+  // up being invalidated, and invalidate ourselves in that case.
----------------
I'd move everything before the "However" to a comment on the PAC line. The rest makes sense here.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61284





More information about the llvm-commits mailing list