[PATCH] D135387: [LoopPeel] Allow to bypass profitability checks. NFC

Thu Oct 6 13:07:37 PDT 2022

anna added a comment.

In D135387#3840832 <https://reviews.llvm.org/D135387#3840832>, @nikic wrote:

> As the comment indicates, this is not entirely a profitability check: The current peeling code does not correctly update branch weights if this condition is not satisfied. D134803 <https://reviews.llvm.org/D134803> implements the necessary support and removes this check entirely.

Ah, I only saw the need for the profitability check. Note that we still needed the profitability check downstream (the comment just piggybacked on the fact that there was missing functionality) so removing it entirely in D134803 <https://reviews.llvm.org/D134803> would break performance for us.

> I also don't get how you plan to use this flag -- after all peelLoop() also contains a canPeel() assertion, so you wouldn't be able to call to call it anyway.

Yes, that's right. I didn't see the `canPeel` assertion. I just had a separate API downstream for profitability.  I think it makes sense to introduce the profitability API as such upstream and separate out the legality from profitability.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135387