[PATCH] D49425: [MemorySSAUpdater] Update Phi operands after trivial Phi elimination

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea added a comment.

In https://reviews.llvm.org/D49425#1167032, @efriedma wrote:

> If the bitcode is crashing but the textual IR isn't, you're probably getting bitten by use-list ordering.  You can use the preserve-ll-uselistorder option for "opt" to preserve it in IR.


Yeap, that worked.

Waiting for @george.burgess.iv's comments on the following to proceed accordingly.

In https://reviews.llvm.org/D49425#1166187, @labrinea wrote:

> > Not sure I understand the question. Do you mean "why do we cache the Phi we've just created if we're visiting a block for the second time?" If so, the cache will automatically track the removal if we do ultimately end up removing the phi. If not, we'll fill the Phi in when we wind our stack back to the bit that populates it, so the second cache insertion should be a nop anyway..
>
> Imagine we call `getPreviousDefRecursive` on a BB with > 1 pred. Assume there's no Phi yet. We mark the BB as visited and start collecting PhiOps with recursive calls via `getPreviousDefFromEnd`. By the time we reach BB again we create an empty Phi and cache it for the first time (line 62). As all the PhiOps are collected we call `tryRemoveTrivialPhi` which might have replaced the Phi with another MemoryAccess. Then we try to cache that MemoryAccess with the same BB key (line 110), which should return false.
>
> > Should this be a TrackingVH<MemoryAccess> instead? That way, we don't need the "Phi ops may be out of date" loop below.
>
> Works, but still it's not clear to me why. What happens if the PhiOp is a deleted Phi itself? Do we still keep the instance of the dead Phi even though MSSA has deleted the corresponding MemoryAccess? How would this work without the updating loop?





https://reviews.llvm.org/D49425