On May 9, 2017, at 9:58 AM, Segher Boessenkool <seg...@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
>
