On Wed, 2020-12-02 at 17:44 +0800, Kewen.Lin via Gcc-patches wrote:
> Hi,
> 
> This patch is to use paradoxical subreg instead of
> zero_extend for promoting QI/HI to SI/DI when we
> want to construct one vector with these modes.
> Since we do the gpr->vsx movement and vector merge
> or pack later, the high part is useless and safe to
> use paradoxical subreg.  It can avoid useless rlwinms
> generated for signed cases.
> 
> Bootstrapped/regtested on powerpc64le-linux-gnu P9.
> 
> Is it ok for trunk?

Mostly cosmetic review.  comments sprinkled below.
thanks
-Will

> 
> BR,
> Kewen
> ------
> gcc/ChangeLog:
> 
>       * config/rs6000/rs6000.c (rs6000_expand_vector_init): Use
>       paradoxical subreg instead of zero_extend for QI/HI promotion
>       when doing QI/HI vector init.

A bit long, but OK with me. :-)

> 
> gcc/testsuite/ChangeLog:
> 
>       * gcc.target/powerpc/pr96933-1.c: Adjusted to check no rlwinm.
>       * gcc.target/powerpc/pr96933-2.c: Likewise.

Ok.  (I'd hope a few more extend instructions would be eliminated, but
this only covers the tests that explicitly looked/didn't look for them, so OK).



> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index f33fca3982a..9c084b055b8 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -6793,17 +6793,8 @@ rs6000_expand_vector_init (rtx target, rtx vals)

I note that the code changes that follow here are within the code block 

  if (TARGET_DIRECT_MOVE && (mode == V16QImode || mode == V8HImode))
    {

This is implied per the patch description, but not obvious from the context of 
the changes here.   (OK).


>        /* Force the values into word_mode registers.  */
>        for (i = 0; i < n_elts; i++)
>         {
> -         rtx tmp = force_reg (GET_MODE_INNER (mode), XVECEXP (vals, 0, i));
> -         if (TARGET_POWERPC64)
> -           {
> -             op[i] = gen_reg_rtx (DImode);
> -             emit_insn (gen_zero_extendqidi2 (op[i], tmp));
> -           }
> -         else
> -           {
> -             op[i] = gen_reg_rtx (SImode);
> -             emit_insn (gen_zero_extendqisi2 (op[i], tmp));
> -           }
> +         rtx tmp = force_reg (inner_mode, XVECEXP (vals, 0, i));
> +         op[i] = simplify_gen_subreg (Pmode, tmp, inner_mode, 0);
>         }
> 
>        /* Take unsigned char big endianness on 64bit as example for below
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr96933-1.c 
> b/gcc/testsuite/gcc.target/powerpc/pr96933-1.c
> index 3b63865b3b8..71d72084413 100644
> --- a/gcc/testsuite/gcc.target/powerpc/pr96933-1.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr96933-1.c
> @@ -13,3 +13,4 @@
>  /* { dg-final { scan-assembler-times {\mvpkudum\M} 12 } } */
>  /* { dg-final { scan-assembler-not {\mstb\M} } } */
>  /* { dg-final { scan-assembler-not {\msth\M} } } */
> +/* { dg-final { scan-assembler-not {\mrlwinm\M} } } */
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr96933-2.c 
> b/gcc/testsuite/gcc.target/powerpc/pr96933-2.c
> index cef8fbd4f35..9fa15125d8d 100644
> --- a/gcc/testsuite/gcc.target/powerpc/pr96933-2.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr96933-2.c
> @@ -13,3 +13,4 @@
>  /* { dg-final { scan-assembler-times {\mxxpermdi\M} 4 } } */
>  /* { dg-final { scan-assembler-not {\mstb\M} } } */
>  /* { dg-final { scan-assembler-not {\msth\M} } } */
> +/* { dg-final { scan-assembler-not {\mrlwinm\M} } } */

Ok.
Thanks
-Will



Reply via email to