[LoopUnroll] Use addClonedBlockToLoopInfo to clone the top level loop (NFC)

[fhahn]

Summary

rL293124 added the necessary infrastructure to properly add the cloned
top level loop to LoopInfo, which means we do not have to do it manually
in CloneLoopBlocks.

@mkuper sorry for not pointing this out during my review of D29156, I just
realized that today.

[fhahn]

Put NewLoop decl in if.

[mkuper]

Nice cleanup, thanks - addClonedBlockToLoopInfo() will indeed add the top-level itself, I didn't notice this when I wrote the patch.
But I think you still need to know whether you actually created a remainder loop, see inline comment.

lib/Transforms/Utils/LoopUnrollRuntime.cpp

389–391

Are you sure you want to do this if NewLoops[L] is the parent loop? I think this code is meant to only disable unrolling on the remainder loop.

[fhahn]

Fixed check

lib/Transforms/Utils/LoopUnrollRuntime.cpp

389–391

Ah good spot. We should check CreateRemainderLoop I think.

[mkuper]

Sorry, I missed the fixed revision.
LGTM.

lib/Transforms/Utils/LoopUnrollRuntime.cpp

static void CloneLoopBlocks(Loop *L, Value *NewIter,
                             LoopBlocksDFS &LoopBlocks, ValueToValueMapTy &VMap,
                             DominatorTree *DT, LoopInfo *LI) {
   StringRef suffix = UseEpilogRemainder ? "epil" : "prol";
   BasicBlock *Header = L->getHeader();
   BasicBlock *Latch = L->getLoopLatch();
   Function *F = Header->getParent();
   LoopBlocksDFS::RPOIterator BlockBegin = LoopBlocks.beginRPO();
   LoopBlocksDFS::RPOIterator BlockEnd = LoopBlocks.endRPO();
-  Loop *NewLoop = nullptr;
   Loop *ParentLoop = L->getParentLoop();
-  if (CreateRemainderLoop) {
-    NewLoop = new Loop();
-    if (ParentLoop)
-      ParentLoop->addChildLoop(NewLoop);
-    else
-      LI->addTopLevelLoop(NewLoop);
-  }
-
   NewLoopsMap NewLoops;
-  if (NewLoop)
-    NewLoops[L] = NewLoop;
-  else if (ParentLoop)
+  NewLoops[ParentLoop] = ParentLoop;
+  if (!CreateRemainderLoop)
     NewLoops[L] = ParentLoop;
 
   // For each block in the original loop, create a new copy,
   // and update the value map with the newly created values.
   for (LoopBlocksDFS::RPOIterator BB = BlockBegin; BB != BlockEnd; ++BB) {
     BasicBlock *NewBB = CloneBasicBlock(*BB, VMap, "." + suffix, F);
     NewBlocks.push_back(NewBB);
-   
     // If we're unrolling the outermost loop, there's no remainder loop,
     // and this block isn't in a nested loop, then the new block is not
     // in any loop. Otherwise, add it to loopinfo.
     if (CreateRemainderLoop || LI->getLoopFor(*BB) != L || ParentLoop)
       addClonedBlockToLoopInfo(*BB, NewBB, LI, NewLoops);
 
     VMap[*BB] = NewBB;
     if (Header == *BB) {
if (!CreateRemainderLoop) {
       BasicBlock *NewLatch = cast<BasicBlock>(VMap[Latch]);
       idx = NewPHI->getBasicBlockIndex(Latch);
       Value *InVal = NewPHI->getIncomingValue(idx);
       NewPHI->setIncomingBlock(idx, NewLatch);
       if (Value *V = VMap.lookup(InVal))
         NewPHI->setIncomingValue(idx, V);
     }
   }
-  if (NewLoop) {
+  if (CreateRemainderLoop) {
+    Loop *NewLoop = NewLoops[L];
+    assert(NewLoop && "L should have been cloned");
     // Add unroll disable metadata to disable future unrolling for this loop.
     SmallVector<Metadata *, 4> MDs;
     // Reserve first location for self reference to the LoopID metadata node.
     MDs.push_back(nullptr);
     MDNode *LoopID = NewLoop->getLoopID();
     if (LoopID) {
       // First remove any existing loop unrolling metadata.
       for (unsigned i = 1, ie = LoopID->getNumOperands(); i < ie; ++i) {