[PATCH] D147078: [LICM][BPF] Disable hoistMinMax() optimization for BPF target

[Alexei Starovoitov via Phabricator via llvm-commits]

ast added a comment.

In D147078#4246363 <https://reviews.llvm.org/D147078#4246363>, @xbolva00 wrote:

>>> Undoing it in the backend is fragile.
>
> Can you explain it why? or in CodegenPrepare? Why is it fragile?
>
> As a bonus you will handle situation when there is  < min(...) directly in your code.

It's not a bonus. People don't write such code now because the verifier doesn't recognize it.

> Backends must follow some rules given by the design of LLVM, LLVM should not be full of ugly (but quickly done) hacks to make backends happy, and currently we saw no proofs that undo transform is not possible to do it.

Ugly is relative. You're arguing that single line boolean check makes optimizer ugly while suggesting to add hundreds of pattern matching fragile code to the backend. Makes zero sense.

btw hexagon might be another example where hoistMin transformation may hurt performance.
See HexagonHardwareLoops.cpp and computeCount().
The backend has to know the constant end value of the loop to do a HW loop.
hoistMin "optimization" hurts this analysis, since backend is not able to know the upper bound.
It's similar for BPF, but in our case it's the kernel verifier that has to know the upper bound.
My limited understanding of hexagon arch is that inability to use hw loop is not the end of the world. Just slow performance.
Whereas for BPF backend it's full breakage. Because of "optimization" valid C programs get rejected by the kernel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147078