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

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added a comment.

A potential solution at the callsite is to simply not call RAUW if the user is a `PHINode`. In the given example, the replaced phi is simplified to `%p.0 = phi i32* [ %arrayidx, %for.end ]` then replaced with    `%arrayidx = getelementptr inbounds [3 x i32], [3 x i32]* %gc, i64 0, i64 0`, which again triggers a call in AST, updating the value.
Result ends up correct (see D58746 <https://reviews.llvm.org/D58746>).

I'm not sure this is correct in general. And I have no idea if there aren't other such cases in other places.

My opinion would be patch this up the best we can now and remove the caching of the AST as soon as possible. 
Current patch is a perfectly fine "patch up". D58746 <https://reviews.llvm.org/D58746> doesn't appear to break anything either and it should save on a needless alias call when calling `AliasSetTracker::copyValue`.
I don't have a strong preference either way.

I *do* prefer removing the AST caching option, but there will be a compile-time performance penalty if we remove the line doing the caching in LICM now.
Enabling MemorySSA removes the caching and it should have compile-time benefits. But this one needs more extensive testing to increase confidence.


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

https://reviews.llvm.org/D58560