[PATCH] D80477: [LoopUnroll] Support loops with exiting block that is neither header nor latch.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopUnroll.cpp:309
+  // (1) single latch; and
+  // (2a) latch is an exiting block; or
+  // (2b) latch is unconditional and there exists a single exiting block.
----------------
I think for 2) it should be sufficient to state that we require a single exiting block. Unless I am missing something, that also implies that the latch must either be unconditional or the exiting block.


================
Comment at: llvm/lib/Transforms/Utils/LoopUnroll.cpp:319
+  BranchInst *ExitingBI = nullptr;
+  bool LatchIsExiting = L->isLoopExiting(LatchBlock);
+  if (LatchIsExiting)
----------------
We really only have to know about whether the latch is exiting when it comes to patching up the latches (around line 716). Might be good to move it down there and just use `getExitingBlock` unconditionally. It would also require moving incrementing the stats increment (NumUnrolledNotLatch++), but that makes sense further down below as well.


================
Comment at: llvm/lib/Transforms/Utils/LoopUnroll.cpp:326
     LLVM_DEBUG(dbgs() << "  Can't unroll; loop not terminated by a conditional "
-                         "branch in the latch or header.\n");
-    return LoopUnrollResult::Unmodified;
-  }
-
-  auto CheckLatchSuccessors = [&](unsigned S1, unsigned S2) {
-    return BI->isConditional() && BI->getSuccessor(S1) == Header &&
-           !L->contains(BI->getSuccessor(S2));
-  };
-
-  // If we have a conditional latch, it must exit the loop.
-  if (BI && BI->isConditional() && !CheckLatchSuccessors(0, 1) &&
-      !CheckLatchSuccessors(1, 0)) {
-    LLVM_DEBUG(
-        dbgs() << "Can't unroll; a conditional latch must exit the loop");
-    return LoopUnrollResult::Unmodified;
-  }
-
-  auto CheckHeaderSuccessors = [&](unsigned S1, unsigned S2) {
-    return HeaderBI && HeaderBI->isConditional() &&
-           L->contains(HeaderBI->getSuccessor(S1)) &&
-           !L->contains(HeaderBI->getSuccessor(S2));
-  };
-
-  // If we do not have a conditional latch, the header must exit the loop.
-  if (BI && !BI->isConditional() && HeaderBI && HeaderBI->isConditional() &&
-      !CheckHeaderSuccessors(0, 1) && !CheckHeaderSuccessors(1, 0)) {
-    LLVM_DEBUG(dbgs() << "Can't unroll; conditional header must exit the loop");
+                         "branch in latch or a single exiting block.\n");
     return LoopUnrollResult::Unmodified;
----------------
might be worth updating the wording to match the checks a bit closer, IIUC : `loop does not have a single exiting block or latch or exiting block not terminated by a branch `


================
Comment at: llvm/lib/Transforms/Utils/LoopUnroll.cpp:806
       if (BB == TermBlock) {
         // The latch is special because we emit unconditional branches in
         // some cases where the original loop contained a conditional branch.
----------------
The comment seems a bit out-of-date now. It would be good to re-phrase it in terms of exiting blocks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80477