[PATCH] D43822: [CloneFunction] Support BB == PredBB in DuplicateInstructionsInSplit.

[Anna Thomas via Phabricator via llvm-commits]

anna accepted this revision.
anna added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: lib/Transforms/Utils/CloneFunction.cpp:814
   // mapping and using it to remap operands in the cloned instructions.
-  for (; StopAt != &*BI; ++BI) {
+  // Stop once we see the terminator too. This covers the case where BB's
+  // terminator gets replaced and StopAt == BB's terminator.
----------------
fhahn wrote:
> anna wrote:
> > Basically, StopAt was pointing to the old terminator of BB before it was replaced. `SplitEdge` does not update BB's terminator. So, where exactly, is the update happening for BB's terminator now?
> > 
> > If PredBB == BB, then BB's terminator should be updated from "br i1 <cond> BB, otherBB" to "br i1 <cond> NewBB, otherBB".
> > 
> The case that caused the crash was PredBB == BB and BB has an unconditional branch as a terminator. Then we split BB at the terminator, which replaces the terminator of BB (relevant line in SplitEdge https://reviews.llvm.org/diffusion/L/browse/llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp;326450$273 , BasicBlock::splitBasicBlock, which is used by SplitBlock https://reviews.llvm.org/diffusion/L/browse/llvm/trunk/lib/IR/BasicBlock.cpp;326450$372)
> 
> 
yes, that makes sense.


https://reviews.llvm.org/D43822