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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D113442#3282412 <https://reviews.llvm.org/D113442#3282412>, @LuoYuanke wrote:

>> 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
>
> I think the integer is also NOT optimal with AVX512 target. See https://godbolt.org/z/Wahef3b3v. For _mm_mask_add_epi64() and _mm_mask_sub_epi64(), it eventually generate extra instructions. But for integer mul, or, and, xor and shl in the test case the InstCombinePass doesn't transform it, so it still generate good code.
>
> I still don't understand what's the benefit to move select instruction before the binary operation (add, sub, mul ...).  Can someone indicate why it is profitable? If it is profitable for general case, I think we can call TTI interface to ask backend if the transform is profitable and then make the decision to transform. We may add below interfaces in TTI.
>
>   +
>   +  bool isLegalMaskedFAdd(Type *DataTye);
>   +  bool isLegalMaskedFSub(Type *DataTye);
>   +  bool isLegalMaskedFMul(Type *DataTye);
>   +  bool isLegalMaskedFDiv(Type *DataTye);
>   +

I believe at least one reason for the int combine was because it reduces the number of uses of the input.

ARM and RISCV both reverse it in the backend for scalars. Though they should probably be using FREEZE to do it.

X86 could probably reverse it for vectors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113442