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

Mon Sep 13 23:44:30 PDT 2021

pengfei added a comment.

> Do we have any builtins with the same name as gcc but different operands/behaviours? Those are the only ones that I'd be worried about.

I think it's rare in existing intrinsics. 1) The builtins are always straightforward passed the arguments in intrinsics in the same order; 2) We do have a few that the the order between builtins and arguments is different, I checked several manually and didn't find any problem.
I think it's a good question for new enabled ISAs, I will check FP16 when GCC patches landed.

> Using a different name in clang for something gcc also has is kinda silly. But I guess we’re worse at this then I thought.
>
> Using the same name and having different behavior should be avoided if the it won’t give a compile error.

We may diversify a few on the names with GCC in FP16, I think we can fix them once GCC landed as a nice to have.

> 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

I think this is a simply approach if we want to keep the builtin in a special C, A, B order. But I don't prefer this way because we need to swapped the order 3 times, intrinsic to builtin, builtin to SDNode, SDNode to MI. Which may result confusion for future developer.

> It seems in this patch the builtins interface is aligned to intrinsics interface. Since AVX512FP16 is pretty new, I assume nobody is using the GCC builtin. Can we ask GCC guys change their builtin interface?

I think we should make an agreement with them. Just found we are using different order on cfmul builtins already.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109658