[PATCH] D63921: [Loop Peeling] Add support for peeling of loops with multiple exits

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:445
   BranchInst *LatchBR = cast<BranchInst>(NewLatch->getTerminator());
-  unsigned HeaderIdx = (LatchBR->getSuccessor(0) == Header ? 0 : 1);
-  LatchBR->setSuccessor(HeaderIdx, InsertBot);
-  LatchBR->setSuccessor(1 - HeaderIdx, Exit);
+  for (unsigned idx = 0; idx < LatchBR->getNumSuccessors(); ++idx)
+    if (LatchBR->getSuccessor(idx) == Header) {
----------------
Maybe assign LatchBR->getNumSuccessors() to a variable, to avoid re-evaluating it?


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:480
   // a value coming into the header.
-  for (BasicBlock::iterator I = Exit->begin(); isa<PHINode>(I); ++I) {
-    PHINode *PHI = cast<PHINode>(I);
-    Value *LatchVal = PHI->getIncomingValueForBlock(Latch);
-    Instruction *LatchInst = dyn_cast<Instruction>(LatchVal);
-    if (LatchInst && L->contains(LatchInst))
-      LatchVal = VMap[LatchVal];
-    PHI->addIncoming(LatchVal, cast<BasicBlock>(VMap[Latch]));
-  }
+  for (auto It : ExitEdges)
+    for (BasicBlock::iterator I = It.second->begin(); isa<PHINode>(I); ++I) {
----------------
Can you use a more descriptive variable name here? Same at line 620


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:481
+  for (auto It : ExitEdges)
+    for (BasicBlock::iterator I = It.second->begin(); isa<PHINode>(I); ++I) {
+      PHINode *PHI = cast<PHINode>(I);
----------------
Can you use It.second->phis() instead?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63921/new/

https://reviews.llvm.org/D63921