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

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv added a comment.

> 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

Thanks for the clarification. I don't think that can happen, since the only way we can remove a `Phi` is if we call `tryRemoveTrivialPhi` directly on it (which doesn't happen), or if we recursively call it on said Phi. We only recurse to `User`s of arbitrary MemoryAccesses, and the blank Phis we create are `User`s of nothing until we fill their operands in.

If you'd like, you're welcome to add something like `assert(Inserted || CachedPreviousDef[BB] == Result);` to be sure of this, but like said, I don't see how it can happen.

> Works, but still it's not clear to me why. What happens if the PhiOp is a deleted Phi itself?

MSSA Phi deletion requires replacing all `Use`s (...in the LLVM sense of Use, not MemoryUse) of the to-be-deleted Phi with another MemoryAccess prior to us actually `delete`ing the Phi. As a part of this replacement, the TrackingVHes will all automatically get pointed at this new MemoryAccess.


https://reviews.llvm.org/D49425