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

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147078