[PATCH] D48457: [LoopUnswitch]Fix comparison for DomTree updates.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopUnswitch.cpp:945
     SmallVector<DominatorTree::UpdateType, 3> Updates;
-    if (TrueDest != OldBranchParent)
       Updates.push_back({DominatorTree::Insert, OldBranchParent, TrueDest});
----------------
kuhar wrote:
> If it's illegal to loop back to preheader, maybe there should be an assert for that?
I'm not sure we can get that info in this method. This is essentially an util, independent of the loopunswitch logic at the callsite. 


================
Comment at: lib/Transforms/Scalar/LoopUnswitch.cpp:947
       Updates.push_back({DominatorTree::Insert, OldBranchParent, TrueDest});
-    if (FalseDest != OldBranchParent)
+    if (FalseDest != OldBranchSucc)
       Updates.push_back({DominatorTree::Insert, OldBranchParent, FalseDest});
----------------
kuhar wrote:
> Is it possible that TrueDest == FalseDest? In this is the case, we should check it here as well.
This should not be possible.
The logic is contained to LoopUnswitch and the two callsites for this method each have a new block as the TrueDest and an existing block as FalseDest. I can add an assert (TrueDest != FalseDest), if you think it would be helpful.


Repository:
  rL LLVM

https://reviews.llvm.org/D48457