[PATCH] D44564: [BasicAA] Relax restriction on PHI node handling.

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.

In https://reviews.llvm.org/D44564#1083070, @john.brawn wrote:

> In https://reviews.llvm.org/D44564#1079954, @dberlin wrote:
>
> > You can't really have something it uses do caching, because there is no way to "invalidate" BasicAA.
>
>
> But it looks like that's already happening?


Not really intentionally, it just grew this way :-)

> Looking at BasicAA::run it uses DominatorTreeAnalysis, and BasicAAResult::invalidate returns true if DominatorTreeAnalysis is invalidated.

Right, but that doesn't help your problem below either :)

> Also, with regards to invalidation, how is this supposed to happen in the middle of a pass?

Which?
BasicAA invalidation or PhiAA invalidation?
Because you actually have the same problem below if the assumptions or dominator tree change in the middle of GVN.
Nothing will invalidate BasicAA in that case right now.
In that regard, we are not actually worse off.

> What's happening in GVN currently is:
> 
> - A load is determined to be partially redundant
> - A chain of phis is inserted, which ends up using the load
> - The load is removed and all uses replaced with the end of the phi chain, which causes the phi chain to contain a loop with only one underlying value
> - GVN then goes on to analyze another load which used to use the value of the first load but which now uses this phi chain
> - GVN asks MemoryDependenceAnalysis about the dependencies of this load
> - MemoryDependenceAnalysis asks AAResults
> - AAResults asks BasicAAResult
> - BasicAAResult now has to figure out this phi chain
> 
>   If we have some analysis which looks at phis and caches the results (either PhiAnalysis calculating the underlying values of phis, or PhiAA calculating what phis alias with) then it needs to be recalculated somewhere between GVN removing the load and AAResults being asked about the phi that's been inserted.

Again, besides BasicAA *already* having this problem (and it just happens that you aren't touching a part that uses assumes or domtree), memdep has this problem too, and tries to provide an incremental invalidation/update interface.
Similarly, we would do the same for PhiAA.
In this case, MemDep's invalidation interface would invalidate PhiAA.
This also assumes a bunch of stuff (for example, that more powerful analysis can't resolve this regardless of the load being there, etc)

>   How is this supposed to happen? 

Nothing here, in any way, is an entirely well designed and architected complete solution to these problems :)
You are welcome to suggest one :)


Repository:
  rL LLVM

https://reviews.llvm.org/D44564