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

Wed Apr 5 09:39:17 PDT 2023

ast added a comment.

In D147078#4245457 <https://reviews.llvm.org/D147078#4245457>, @mkazantsev wrote:

> 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.

There was no such claim. It's unprofitable for BPF.

> It would be a good general improvement too.

'general improvement' is true only for a subset of CPUs. It's not an improvement for BPF "CPU".

> Optimizer is not committed to know about such kind of issues of backends.

This is a false statement. Optimizer has to work hand and hand with the backend.
Several backends have loop insns and backends go out of their way to pattern match loops in the backend.
When optimizer does something "smart" claiming it's a valid llvm IR it may screw up the analysis and valid C programs would get rejected because of optimizer.
This case is not loop related, but similar in a sense that optimizer hurts a backend.

> So please, if you want to move ahead, please find the reason of your performance degradation and describe it here.

Performance degradation on hypothetical x86 cpu was an illustration of problem. The real problem is unprofitable transformation.
Undoing it in the backend is fragile. It may or may not work and users writing programs in C will have no idea why their valid C code broke.
Therefore backend has to stop optimizer performing unprofitable transformations.
This is not the only unprofitable transformation. See Andrii's examples of other transformations that we need to disable for BPF backend specifically.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147078