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

[Alexei Starovoitov via Phabricator via llvm-commits]

ast added a comment.

In D147078#4240301 <https://reviews.llvm.org/D147078#4240301>, @nikic wrote:

> In D147078#4240289 <https://reviews.llvm.org/D147078#4240289>, @fhahn wrote:
>
>> I think adding a flag doesn't address the concern that were raised about making this behaviour target-dependent in general. I don't feel super strongly about the issue, so if other people in general think we should add a way to disable this transform, this is fine with me.
>>
>> But I think it would be better to undo the transform if needed. There is no guarantee that LICM is the only source of this pattern and it's generally considered that backends should support any form of valid LLVM IR.
>
> Agreed. In some ways this is even worse than the TTI hook because now you're modifying a cl::opt option from TTI, which is not how cl::opt is supposed to be used. This means that for example if you use BPF and some other target in the same process, this option is going to leak across them, not to mention that the modification is probably not thread-safe.
>
> The only proper way to address this is via an undo transform.

It's a valid LLVM IR, but not a profitable optimization.
The backend has all the rights to tell optimizer that certain transformations are unprofitable and should not be applied.
Consider BPF CPU as a smart CPU. Unlike traditional CPUs that do what compilers tell them, BPF CPU understands the code it is going to execute.
In this particular case we're dealing with:
for (i; i < var && i < const_val; i++)

BPF CPU is smart enough to understand that 'i' will not be bigger than const_val. It can check that array[i] within bounds and so on.
Imagine smart x86 CPU that can infer similar information from analyzing the asm code and do HW prefetch of array[0 ... const_val].
The problem with transformation:
min_var = min(var, const_cal);
for (i; i < min_var; i++)
is that CPU no longer considers 'i' to be bounded.
It's causing BPF CPU to lose information about 'i' and would hurt performance of hypothetical smart x86 CPU.

Backends absolutely need knobs to control profitability of transformations. Undoing optimization in the backend is not practical. In many cases it's not possible to undo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147078