[PATCH] D59315: [AliasAnalysis] Second prototype to cache BasicAA / anyAA state.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: lib/Analysis/MemorySSA.cpp:1158-1163
+  // Alternative to modifying AA in place: create new AAResults, destroy right
+  // after. With this alternative, the following APIs go away:
+  // AAResults::setAAToBatch(), AAResults::setAAToSingle(),
+  // AAQueryInfo::reset().
+  // However this alternative needs to add a way to copy all AAs, or use teh
+  // move constructor and re-move the AAs back after building MemorySSA.
----------------
I think using an object to model a batch of queries is going to be significantly cleaner long term. As mentioned in the discussion, in part this is because I think we'll want to have this as an actual analysis that simply is invalidated *much* more eagerly than the "normal" AA analyses (by any change to the IR at all).

To address the move issue -- my suggestion would be to have it *wrap* the AAs in place rather than moving them. I'm not seeing why this won't work? Maybe just need more explanation here.


================
Comment at: lib/Analysis/MemorySSA.cpp:1164-1169
+  // We must also tell the walker about the new AAResults,
+  // because currently the ClobberWalker stores a reference to AA, and it's
+  // being populated with the AA set before calling buildMemorySSA.
+  // e.g. add a call to WalkerBase->updateAA(this->AA); after buildMemorySSA(),
+  // and add an updateAA(AA) in ClobberWalker, and store a pointer to AA
+  // instead.
----------------
This seems a bit weird to me... I would hope that there is a better way to plumb the batched AA wrapper into these APIs, but maybe I don't understand why its difficult?

However, it does raise a specifically interesting point: the walkers should use the batched interface here, but likely *not* use the batched interface after MemorySSA has been built as at that point the IR is likely changing and the batched results are no longer valid.

This seems like an important distinction that should be made more explicit. Maybe the walkers that the rest of the code use should be distinct from those used to build MemorySSA initially?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59315