[PATCH] D30632: [LoopUnrolling] Fix loop size check for peeling

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:78
   // invariant instead of this Phi.
-  if (auto *BackEdge = L->getLoopLatch()) {
-    BasicBlock *Header = L->getHeader();
-    // Iterate over Phis to find one with invariant input on back edge.
-    bool FoundCandidate = false;
-    PHINode *Phi;
-    for (auto BI = Header->begin(); isa<PHINode>(&*BI); ++BI) {
-      Phi = cast<PHINode>(&*BI);
-      Value *Input = Phi->getIncomingValueForBlock(BackEdge);
-      if (L->isLoopInvariant(Input)) {
-        FoundCandidate = true;
-        break;
+  if (LoopSize <= UP.Threshold) {
+    if (auto *BackEdge = L->getLoopLatch()) {
----------------
mkuper wrote:
> mkazantsev wrote:
> > mkuper wrote:
> > > I would combine this into a single if with a &&, rather than two nested ones.
> > We can't combine it with variable's declaration, and declaring the latch externally looks quite ugly.
> I think having another level of nesting here is worse than declaring the latch externally.
> I'd suggest either (1) declaring it externally, or (2) pull this entire thing into a helper function, in which case you can have an early exit.
I just realized that getLoopPatch never returns null, and this is ensured within isLoopSimplifyForm in canPeel method. So I will replace the condition here and make an assert for latch.


https://reviews.llvm.org/D30632