[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