[PATCH] D109658: [X86][FP16] Change the order of the operands in complex FMA intrinsics to allow swap between the mul operands.

Sun Sep 12 23:42:28 PDT 2021

craig.topper added a comment.

In D109658#2996714 <https://reviews.llvm.org/D109658#2996714>, @pengfei wrote:

> In D109658#2996412 <https://reviews.llvm.org/D109658#2996412>, @craig.topper wrote:
>
>> Does gcc use the same builtin name? Our general policy is to have the same interface as gcc if we have a builtin. So if gcc has these builtins the should work the same way.
>
> No. We don't sync with GCC on the builtin name during the development. We had a disscussion and decided to not keep them aligned due to 1) target specific builtins are compiler private names that no need to keep it compatible with other compilers; and 2) we already differentiate the target builtins with GCC long ago on the naming, masking etc. Currently, regardless the name, GCC uses the same C, A, B order with our existing implementation. https://gitlab.com/x86-gcc/gcc/-/blob/users/intel/liuhongt/independentfp16_wip/gcc/config/i386/avx512fp16intrin.h#L6672

I thought we were pretty consistent on names with gcc for most of sse and avx and most of avx512. The names aren't completely private occasionally users due try to use them. If we happen to have the same name we should have the same behavior to avoid confusion.

>> For IFMA I think I made them commutable by swapping the operands between the builtin and the internal intrinsic using the handling for the X86IntrinsicTable
>
> Is this one https://github.com/llvm/llvm-project/commit/80c8b80919e0049da32f018d98e4d75ff562cfa8? Do you mean I should add `Commutative` in IntrinsicsX86.td too? What's this flag used for? I saw we only add them in a few intrinsics.

I was referring to 47e14ead54d229aa8ac5436594e91bfb8943230e for AVX512IFMA

The flag in IntrinsicsX86.td is used if the int_x86_* is used in the isel pattern, but I don't know if we have any of those any more.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109658