Re: [PATCH, rs6000] Fix vec_xl and vec_xst intrinsics for P8

[Bill Schmidt <wschmidt at linux dot vnet dot ibm dot com>]

On May 9, 2017, at 9:58 AM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> 
> Hi!
> 
> On Thu, May 04, 2017 at 04:35:10PM -0500, Bill Schmidt wrote:
>> In an earlier patch, I changed vec_xl and vec_xst to make use of new
>> POWER9 instructions when loading or storing vector short/char values.
>> In so doing, I failed to enable the existing instruction use for
>> -mcpu=power8, so these were no longer considered valid by the compiler.
>> Not good.
>> 
>> This patch fixes the problem by using other existing built-in definitions
>> when the POWER9 instructions are not available.  I've added a test case
>> to improve coverage and demonstrate that the problem is fixed.
>> 
>> Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
>> regressions.  Is this ok for trunk?
> 
> Yes, thanks!  One nit:
> 
>> --- gcc/config/rs6000/rs6000.c	(revision 247560)
>> +++ gcc/config/rs6000/rs6000.c	(working copy)
>> @@ -18183,6 +18183,17 @@ altivec_init_builtins (void)
>>       def_builtin ("__builtin_vsx_st_elemrev_v16qi",
>> 		   void_ftype_v16qi_long_pvoid, VSX_BUILTIN_ST_ELEMREV_V16QI);
>>     }
>> +  else
>> +    {
>> +      rs6000_builtin_decls[(int)VSX_BUILTIN_LD_ELEMREV_V8HI]
>> +	= rs6000_builtin_decls[(int)VSX_BUILTIN_LXVW4X_V8HI];
> 
> There should be a space after the cast operators.

OK, will fix.  Thanks for the review!

I forgot to ask -- this fix is needed for GCC 6 and 7 as well.  Is this ok for backport
after the usual burn-in?

Thanks,
Bill
> 
> 
> Segher
>