[PATCH] D93686: [LoopUnroll] Fix a crash

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopUnroll.cpp:404
+        dbgs() << "Can't unroll; a conditional latch must exit the loop");
+    return LoopUnrollResult::Unmodified;
+  }
----------------
skatkov wrote:
> fhahn wrote:
> > Does this need updating? At this point, we could have peeled the loop and thus change it I think. 
> > 
> > Also, by moving the early exit to after peeling, we now expose more cases to peeling. Can peeling handle the case of the early exit properly?
> I'm sorry I do not follow what you mean here. Could you please elaborate on what should be changed?
> 
> The only earlier return we do not do now for peeling is
> !LatchBI || (LatchBI->isConditional() && !LatchIsExiting)
> 
> Loop peeling has its own guard canPeel() which is also triggered in computePeelCount.
> Peeling guard requires that latch is a conditional exiting block.
> So nothing new we are introducing.
> Loop peeling has its own guard canPeel() which is also triggered in computePeelCount.
> Peeling guard requires that latch is a conditional exiting block.

If we have a similar check for peeling everything is fine, thanks for checking! Can you add an assertion here that we did not peel, so we can catch the case early when we need to update the returned status?


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

https://reviews.llvm.org/D93686