[PATCH] D108108: [LoopPeel] Allow peeling with multiple unreachable-terminated exit blocks.

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopPeel.cpp:105
+  // The latch must either be the only exiting block or all non-latch exit
+  // blocks are either have an deopt or unreachable terminator.
+  return all_of(Exits, [](const BasicBlock *BB) {
----------------
reames wrote:
> nikic wrote:
> > reames wrote:
> > > Can you add something to this comment to indicate that a) this is a proxy for a strong profile prediction of untaken, and b) this is a profitability check not a legality check?
> > I would really appreciate an explanation of why we want this heuristic... it's not really obvious to me why this is only beneficial if the other exits are likely not taken.
> I don't know this is Florian's answer, but mine would be: incrementalism and minimizing blast radius.  We may eventually want to generalize further, but one step at a time.  
I guess the problem is that LoopPeeling cannot update branch weights for other branches than latch.

If we can teach to LoopPeeling to update branch weights for non latch branches - we may remove this restriction however it does not look easy task.

Branch weights to deopt and unreachable is overwritten to be smallest one, so its update is simple (no update).

This is how I remember a problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108108