Re: [PATCH] rs6000: Fix vector_set_var_p9 by considering BE [PR108807]

[Segher Boessenkool <segher@kernel.crashing.org>]

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? :-)

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 :-)

Patch looks good.  Thanks!


Segher