[PATCH] D51563: [MemorySSA] Update MemoryPhi wiring for block splitting to consider if identical edges were merged.
Alina Sbirlea via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 7 13:45:09 PDT 2018
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
More information about the llvm-commits
mailing list