rs6000: allow split vsx_stxvd2x4_le_const after RA[pr116030]

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

Jiufu Guo <guojiufu@linux.ibm.com> writes:

> 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"))]
>    "!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" } */

Is -save-temps needed here?

> +
> +/* 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;
> +}

Hi,

Sam James <sam@gentoo.org> writes:

> Jiufu Guo <guojiufu@linux.ibm.com> writes:
>
>> 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"))]
>>    "!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" } */
>
> Is -save-temps needed here?

Thanks for catching this! We could remove this option for this patch.

BR,
Jeff (Jiufu) Guo.
>
>> +
>> +/* 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;
>> +}

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;
> +}