[PATCH] D32801: [RuntimeLoopUnroller] Add assert that we dont unroll non-rotated loops

[Davide Italiano via Phabricator via llvm-commits]

davide added inline comments.


================
Comment at: lib/Transforms/Utils/LoopUnrollRuntime.cpp:515-521
+  // Cloning the loop basic blocks (`CloneLoopBlocks`) requires that one of the
+  // targets of the Latch be the single exit block out of the loop. This needs
+  // to be guaranteed by the callers of UnrollRuntimeLoopRemainder.
+  BranchInst *LatchBR = cast<BranchInst>(Latch->getTerminator());
+  assert(LatchBR->getSuccessor(0) == Exit ||
+         LatchBR->getSuccessor(1) == Exit && "loop latch successor should be "
+                                             "exit block!");
----------------
anna wrote:
> davide wrote:
> > This seems like a natural extension of what I did.
> > I think the fact that the other successor is the header is given by the definition of a latch, so we should be fine with this.
> Thanks, yes. I had spotted this implicit dependency (latch's successor is an exit) in the runtime unroller's clone function and was wondering if we can have such a situation where the dependency is not true. Sanjoy had pointed me to your fix (and test case) in the loop unroller.
Side note: ideally I'd like to teach the unroller to handle more complicated loops in the future rather than bailing out, but that would require more time than the one I can devote to the cause for the next 3 months at least. 
Thanks for your patch. Feel free to land.


https://reviews.llvm.org/D32801