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

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev requested changes to this revision.
mkazantsev added a comment.
This revision now requires changes to proceed.

Reading through discussion, I now think that we should not go with the option. The reason why performance degraded is not well understood.

If the claim is "this is not a profitable transform in general", I want to see a proof. The tranform is removing number of checks on every loop iteration, so it is surprising to hear that.

If the claim is "it is inhibiting some other optimizations (for example, bount checks elimination)" then I'd like to see an exact example. I made some experiments with it and now am pretty sure that in many cases IndVars, IRCE and other passes that are responsible for range checks elimination do know what is smin and can work with it as well as with two separate checks. Maybe there is a case when they don't support it, and in this case I'd like to see this test and fix it. It would be a good general improvement too.

Finally, if the claim is "this backend works bad with it for some low-level/hardware reasons" - OK, then the right way is to undo the transform. Optimizer is not committed to know about such kind of issues of backends. I also have doubts that it's the case.

So please, if you want to move ahead, please find the reason of your performance degradation and describe it here. Maybe we have just exposed a missing case for IndVarSimplify or something, and it's an opportunity to fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147078