[PATCH] D14909: [X86][FMA] Optimize FNEG(FMUL) Patterns

[Sanjay Patel via llvm-commits]

[I added this to Phab over an hour ago, but it hasn't made it to the list
afaict, so repeating with an email.]

I see what's going on now. This patch needs the AVX512 check in order to
work for 512-bit vectors. There should be at least one more test case
included with this patch to check the 512-bit vector case. Alternatively,
if we want to make 512-bit optimization a separate patch, the predicate
should disallow those types.

So I see 2 bugs here. Test case:

define <16 x float> @test_v4f32_fneg_fmul_16(<16 x float> %x, <16 x
float> %y) #0 {
  %m = fmul <16 x float> %x, %y
  %n = fsub <16 x float> <float -0.0, float -0.0, float -0.0, float
-0.0, float -0.0, float -0.0, float -0.0, float -0.0, float -0.0,
float -0.0, float -0.0, float -0.0, float -0.0, float -0.0, float
-0.0, float -0.0>, %m
  ret <16 x float> %n
}

Bug #1: The AVX-512 attribute in the subtarget should imply hasFMA().
Currently, this isn't true, so if you don't explicitly specify FMA, you
won't get any expected FMA transforms:

$ ./llc fma.ll -o - -mattr=avx512f...   vmulps	%zmm1, %zmm0, %zmm0
vpxord	LCPI0_0(%rip), %zmm0, %zmm0   retq$ ./llc fma.ll -o -
-mattr=avx512f,fma...  vpxord	%zmm2, %zmm2, %zmm2  vfnmadd213ps	%zmm2,
%zmm1, %zmm0  retq

Bug #2: This patch isn't checking for legal types, so it will fire for a
512-bit vector on a target that doesn't support it, and we crash:

$ ./llc fma.ll -o - -mattr=fma  Do not know how to custom type
legalize this operation!  UNREACHABLE executed at
/Users/spatel/myllvm/llvm/lib/Target/X86/X86ISelLowering.cpp:19880!

This would also happen if the test case had an illegal type for any target;
eg <17 x float>.

I think the best fix for #2 is to explicitly check for the supported types
for a given subtarget rather than waiting for type legalization and then
checking the scalar type. This should be a helper function, so we can have
the existing code in PerformFMACombine() use it too.


On Wed, Nov 25, 2015 at 7:29 AM, Simon Pilgrim <llvm-dev at redking.me.uk>
wrote:

> RKSimon marked 3 inline comments as done.
> RKSimon added a comment.
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D14909
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151125/d5224340/attachment.html>