Re: [PATCH] rs6000: allow split vsx_stxvd2x4_le_const after RA[pr116030]

[Kewen.Lin]

Hi,

on 2024/8/21 15:23, Jiufu Guo wrote:
> Hi,
> 
> Previous, vsx_stxvd2x4_le_const_<mode> is introduced for 'split1' pass,
> so it is guarded by "can_create_pseudo_p ()".
> While, it would be possible to match the pattern of this insn during/after
> RA, so this insn could be updated to make it work for split pass after RA.
> 
> Bootstrap&regtest pass on ppc64{,le}.
> Is this ok for trunk?
> 
> BR,
> Jeff (Jiufu) Guo
> 
> 
>       PR target/116030
> 
> gcc/ChangeLog:
> 
>       * config/rs6000/vsx.md (vsx_stxvd2x4_le_const_<mode>): Allow insn
>       after RA.
> 
> gcc/testsuite/ChangeLog:
> 
>       * gcc.target/powerpc/pr116030.c: New test.
> 
> ---
>  gcc/config/rs6000/vsx.md                    |  9 +++++----
>  gcc/testsuite/gcc.target/powerpc/pr116030.c | 17 +++++++++++++++++
>  2 files changed, 22 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/pr116030.c
> 
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index 27069d070e1..2dd87b7a9db 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -3454,12 +3454,12 @@ (define_insn "*vsx_stxvd2x4_le_<mode>"
>  
>  (define_insn_and_split "vsx_stxvd2x4_le_const_<mode>"
>    [(set (match_operand:VSX_W 0 "memory_operand" "=Z")
> -     (match_operand:VSX_W 1 "immediate_operand" "W"))]
> +     (match_operand:VSX_W 1 "immediate_operand" "W"))
> +   (clobber (match_scratch:VSX_W 2 "=&wa"))]

I think "&" isn't needed here as the input is immediate_operand.

>    "!BYTES_BIG_ENDIAN
>     && VECTOR_MEM_VSX_P (<MODE>mode)
>     && !TARGET_P9_VECTOR
> -   && const_vec_duplicate_p (operands[1])
> -   && can_create_pseudo_p ()"
> +   && const_vec_duplicate_p (operands[1])"
>    "#"
>    "&& 1"
>    [(set (match_dup 2)
> @@ -3472,7 +3472,8 @@ (define_insn_and_split "vsx_stxvd2x4_le_const_<mode>"
>  {
>    /* Here all the constants must be loaded without memory.  */
>    gcc_assert (easy_altivec_constant (operands[1], <MODE>mode));
> -  operands[2] = gen_reg_rtx (<MODE>mode);
> +  if (GET_CODE(operands[2]) == SCRATCH)
> +    operands[2] = gen_reg_rtx (<MODE>mode);
>  }
>    [(set_attr "type" "vecstore")
>     (set_attr "length" "8")])
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr116030.c 
> b/gcc/testsuite/gcc.target/powerpc/pr116030.c
> new file mode 100644
> index 00000000000..ada0a4fd2b1
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr116030.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-mdejagnu-cpu=power8 -Os -fno-forward-propagate 
> -ftrivial-auto-var-init=zero -save-temps" } */

As Sam pointed out, "-save-temps" isn't needed.

Since this test case adopts dfp type, please add one more check:

/* { dg-require-effective-target dfp } */

OK with all these fixed, thanks!

BR,
Kewen

> +
> +/* Verify we do not ICE on the tests below.  */
> +union U128
> +{
> +  _Decimal128 d;
> +  unsigned long long int u[2];
> +};
> +
> +union U128
> +foo ()
> +{
> +  volatile union U128 u128;
> +  u128.d = 0.9999999999999999999999999999999999e+39DL;
> +  return u128;
> +}