Hi Segher,
Thanks for the review!
on 2023/4/3 19:44, Segher Boessenkool wrote:
> Hi!
>
> On Fri, Feb 17, 2023 at 05:55:04PM +0800, Kewen.Lin wrote:
>> As PR108807 exposes, the current handling in function
>> rs6000_expand_vector_set_var_p9 doesn't take care of big
>> endianness. Currently the function is to rotate the
>> target vector by moving element to-be-set to element 0,
>> set element 0 with the given val, then rotate back. To
>> get the permutation control vector for the rotation, it
>> makes use of lvsr and lvsl, but the element ordering is
>> different for BE and LE (like element 0 is the most
>> significant one on BE while the least significant one on
>> LE), this patch is to add consideration for BE and make
>> sure permutation control vectors for rotations are expected.
>
>> --- a/gcc/config/rs6000/rs6000.cc
>> +++ b/gcc/config/rs6000/rs6000.cc
>> @@ -7235,22 +7235,26 @@ rs6000_expand_vector_set_var_p9 (rtx target, rtx
>> val, rtx idx)
>>
>> machine_mode shift_mode;
>> rtx (*gen_ashl)(rtx, rtx, rtx);
>> - rtx (*gen_lvsl)(rtx, rtx);
>> - rtx (*gen_lvsr)(rtx, rtx);
>> + rtx (*gen_pcvr1)(rtx, rtx);
>> + rtx (*gen_pcvr2)(rtx, rtx);
>
> Space before "(" btw, you can fix that at the same time? :-)
>
Good catch, fixed.
> What does "pcvr" mean? You could put that in a short comment?
>
>> + /* Generate one permutation control vector used for rotating the element
>
> Ah. Yeah just "/* Permutation control vector */" for the above one
> prevents all mysteries :-)
One comment line added for gen_* function pointers.
>
> Patch looks good. Thanks!
>
Pushed as r13-6994-gd634e6088f139e, thanks!
BR,
Kewen
