[PATCH] D51563: [MemorySSA] Update MemoryPhi wiring for block splitting to consider if identical edges were merged.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added inline comments.


================
Comment at: lib/Analysis/MemorySSAUpdater.cpp:1013
 void MemorySSAUpdater::wireOldPredecessorsToNewImmediatePredecessor(
-    BasicBlock *Old, BasicBlock *New, ArrayRef<BasicBlock *> Preds) {
+    BasicBlock *Old, BasicBlock *New, ArrayRef<BasicBlock *> Preds,
+    bool IdenticalEdgesWereMerged) {
----------------
george.burgess.iv wrote:
> (I see that practically, the answer is "no". Just curious how strongly we're tailoring this function for the user)
> 
> Is it theoretically reasonable for Preds to contain duplicates of the same `BasicBlock` if `!IdenticalEdgesWereMerged`? The concern being that if I, as the user, call `wireOldPredecessorsToNewImmediatePredecessor(Old, New, {BB, BB}, false);`, we'll only end up moving one instance of BB.
> 
> If this concern is founded, please add an assert that `PredsSet.size() == Preds.size()` if `!IdenticalEdgesWereMerged`, so it's obvious that said case is broken if someone tries to depend on it in the future (which I'm totally cool with, given that, like said, this can't happen at the moment). 
Agreed, added the assert!


Repository:
  rL LLVM

https://reviews.llvm.org/D51563