[PATCH] D58560: [AST] Set 'MayAlias' instead of 'MustAlias' in AliasSets for PHI nodes (bugzilla bug#36801)

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

Spent a bit more time looking, and thinking, and I think I have a clearer picture of what's happening.

Given that a pointer inside a loop is getting rewritten with an arbitrary SSAUpdater transform, we can't really keep the AliasSetTracker up to date given the information from the callback. We can't tell what updates are actually relevant.  And even if we could, the updates might provide additional information about the pointers in question which would allow splitting the alias set.

The PHI node thing is sort of a red herring; it does not matter at all whether the operation that produces or consumes the pointer is a PHI node, in general.  Maybe for certain specific transforms that's true, but more generally it doesn't make sense; consider, for example, LICM itself.  (That particular interaction doesn't actually happen, I think, because we don't run LICM twice in a row on the same loop nest, but other passes could perform similar transforms.)

Given the way this works, what information can we reasonably preserve?  If want to preserve the AliasSetTracker, and we assume that loop passes don't perform any transforms that actually change the aliasing characteristics of a loop (which I'm not sure is really a safe assumption, but we have to assume it to preserve the AliasSetTracker at all), I guess this patch is essentially the right solution... except that the `!(isa<PHINode>(From))` bit doesn't really make any sense, unless we're specifically assuming that the only pass which can trigger copyValue callbacks is LoopRotate.  The only other reasonable alternative is, if we get any updates from SSAUpdater, we completely discard the AliasSetTracker and recompute it later.

Of course, this would all be much more straightforward if LICM used MemorySSA instead of using its crazy hack to cache the AliasSetTracker: MemorySSA is a proper analysis that other passes can accurately preserve.

If we're going to do this as a temporary fix, this needs much better comments explaining when exactly we expect the callback to be called, and why we chose this particular solution.


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

https://reviews.llvm.org/D58560