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

George Burgess IV via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 5 09:25:52 PDT 2018


george.burgess.iv accepted this revision.
george.burgess.iv added a comment.
This revision is now accepted and ready to land.

Thanks!



================
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) {
----------------
(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). 


Repository:
  rL LLVM

https://reviews.llvm.org/D51563





More information about the llvm-commits mailing list