[PATCH] D113442: [InstCombine] Enable fold select into operand for FAdd, FMul, FSub and FDiv.

Sat Jan 29 07:25:33 PST 2022

spatel added a comment.

In D113442#3281741 <https://reviews.llvm.org/D113442#3281741>, @xbolva00 wrote:

> In D113442#3281729 <https://reviews.llvm.org/D113442#3281729>, @LuoYuanke wrote:
>
>> This patch cause regression (https://godbolt.org/z/7WYboTe16) on x86 avx512 target, because avx512 has mask instruction which can match select instruction in ISel pattern match. Move select before fadd cause the pattern match fails. I don't quite understand for the optimization opportunity for this transformation. Could you elaborate an example for it?
>>
>> This patch has side effect on AVX512 ISel. Another example is https://godbolt.org/z/xdr8xs4cb.
>
> Yeah, this patch was commited without checking how this transformation affects x86 codegen. :/ needs to be reverted before LLVM 14.
>
> cc @spatel

Note that this patch makes IR more consistent (FP ops are treated the same as integer ops). So is the integer codegen also not optimal?
https://godbolt.org/z/jd3q6Yq55

If there's still time for clang 14, can we fix x86 codegen rather than revert? There was already a similar bug filed for compares:
https://github.com/llvm/llvm-project/issues/51842

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113442