[LoopUnroll] Use changeToUnreachable()

[nikic]

Summary

When fulling unrolling with a non-latch exit, the latch block is folded to unreachable. Replace this folding with the existing changeToUnreachable() helper, rather than performing it manually.

This is not NFC because we now also drop all the instructions from the unreachable latch block. Previously we would only convert the terminator, and (later) DCE anything that is trivially dead.

[reames]

I have two concerns with this patch.

1. This is moving the code from before the dom tree rewrite to after. Is the dom tree rewrite correct with the changed CFG? From what I can tell, it is, but critical thought here is warranted.
2. Previously a throwing instruction in the last latch block was left in place. If we ever had a case where we computed a trap count from a known throwing instruction, the new behavior is different than the old behavior. I don't know of anything which does this, but I think I'd prefer to be conservative here.

LGTM w/the unreachable moved to the end of the latch block.

[nikic]

In D103340#2787715, @reames wrote:

I have two concerns with this patch.

1. This is moving the code from before the dom tree rewrite to after. Is the dom tree rewrite correct with the changed CFG? From what I can tell, it is, but critical thought here is warranted.

Right, this is part of the motivation here: moving code out of the scope of the tricky manual dom tree update code. I believe that this is correct, because the particular CFG change that is performed here is one that does not affect the DT at all: The removal of an unconditional backedge.

2. Previously a throwing instruction in the last latch block was left in place. If we ever had a case where we computed a trap count from a known throwing instruction, the new behavior is different than the old behavior. I don't know of anything which does this, but I think I'd prefer to be conservative here.

LGTM w/the unreachable moved to the end of the latch block.

Okay, I'll do that, which should make this an NFC change.

llvm/lib/Transforms/Utils/LoopUnroll.cpp

for (unsigned i = 0, e = ExitingBlocks.size(); i != e; ++i) {
       // TODO: Also fold known-exiting branches for non-latch exits.
       if (*KnownWillExit && !LatchIsExiting)
         continue;
 
       SetDest(ExitingBlocks[i], *KnownWillExit, ExitOnTrue);
     }
   }
 
-  // When completely unrolling, the last latch becomes unreachable.
-  if (!LatchIsExiting && CompletelyUnroll) {
-    BranchInst *Term = cast<BranchInst>(Latches.back()->getTerminator());
-    new UnreachableInst(Term->getContext(), Term);
-    Term->eraseFromParent();
-  }
-
   // Update dominators of blocks we might reach through exits.
   // Immediate dominator of such block might change, because we add more
   // routes which can lead to the exit: we can now reach it from the copied
   // iterations too.
   if (DT && ULO.Count > 1) {
     for (auto *BB : OriginalLoopBlocks) {
       auto *BBDomNode = DT->getNode(BB);
       SmallVector<BasicBlock *, 16> ChildrenToUpdate;
for (auto *BB : OriginalLoopBlocks) {
         DT->changeImmediateDominator(ChildBB, NewIDom);
     }
   }
 
   assert(!DT || !UnrollVerifyDomtree ||
          DT->verify(DominatorTree::VerificationLevel::Fast));
 
   DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Lazy);
+
+  // When completely unrolling, the last latch becomes unreachable.
+  if (!LatchIsExiting && CompletelyUnroll)
+    changeToUnreachable(&Latches.back()->front(), /* UseTrap */ false,
+                        PreserveLCSSA, &DTU);
+
   // Merge adjacent basic blocks, if possible.
   for (BasicBlock *Latch : Latches) {
     BranchInst *Term = dyn_cast<BranchInst>(Latch->getTerminator());
     assert((Term ||
             (CompletelyUnroll && !LatchIsExiting && Latch == Latches.back())) &&
            "Need a branch as terminator, except when fully unrolling with "
            "unconditional latch");
     if (Term && Term->isUnconditional()) {

llvm/test/Transforms/LoopUnroll/unroll-unconditional-latch.ll

 ; CHECK-NEXT:    store i32 0, i32* [[PTR_2]], align 4
 ; CHECK-NEXT:    br label [[FOR_LATCH_2:%.*]]
 ; CHECK:       for.latch.2:
 ; CHECK-NEXT:    store volatile i64 2, i64* [[OUT]], align 4
 ; CHECK-NEXT:    [[PTR_3:%.*]] = getelementptr inbounds i32, i32* [[ARG]], i64 3
 ; CHECK-NEXT:    store i32 0, i32* [[PTR_3]], align 4
 ; CHECK-NEXT:    br i1 true, label [[IF_END_LOOPEXIT:%.*]], label [[FOR_LATCH_3:%.*]]
 ; CHECK:       for.latch.3:
-; CHECK-NEXT:    store volatile i64 3, i64* [[OUT]], align 4
 ; CHECK-NEXT:    unreachable
 ;
 
 entry:
   br label %for.header
 
 for.header:                              ; preds = %for.latch, %entry
   %indvars.iv800 = phi i64 [ 0, %entry ], [ %indvars.iv.next801, %for.latch ]