Re: [PATCH] rs6000: avoid ineffective replacement of splitters

2022-08-16 Thread Jiufu Guo via Gcc-patches
Hi,

"Kewen.Lin"  writes:

> Hi Jeff,
>
> on 2022/8/12 14:39, Jiufu Guo via Gcc-patches wrote:
>> Hi,
>> 
>> As a comment in
>> https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599556.html
>> 
>> Those splitters call rs6000_emit_set_const directly, and the replacements
>> are never used.  Using (pc) would be less misleading.
>
> Since the replacements are never used, IMHO this subject doesn't
> quite meet the change.  How about "fix misleading new patterns
> of splitters"?
Thanks for your helpful sugguestion!

BR,
Jeff(Jiufu)
>
>> 
>> This patch pass bootstrap on ppc64 BE and LE.
>> Is this ok for trunk.
>
> This patch is OK w/ or w/o subject tweaked.  Thanks!
>
> BR,
> Kewen
>
>> 
>> BR,
>> Jeff(Jiufu)
>> 
>> gcc/ChangeLog:
>> 
>>  * config/rs6000/rs6000.md: (constant splitters): Use "(pc)" as the
>>  replacements.
>> 
>> ---
>>  gcc/config/rs6000/rs6000.md | 12 +++-
>>  1 file changed, 3 insertions(+), 9 deletions(-)
>> 
>> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
>> index 1367a2cb779..7fadbeef1aa 100644
>> --- a/gcc/config/rs6000/rs6000.md
>> +++ b/gcc/config/rs6000/rs6000.md
>> @@ -7727,11 +7727,7 @@ (define_split
>>[(set (match_operand:SI 0 "gpc_reg_operand")
>>  (match_operand:SI 1 "const_int_operand"))]
>>"num_insns_constant (operands[1], SImode) > 1"
>> -  [(set (match_dup 0)
>> -(match_dup 2))
>> -   (set (match_dup 0)
>> -(ior:SI (match_dup 0)
>> -(match_dup 3)))]
>> +  [(pc)]
>>  {
>>if (rs6000_emit_set_const (operands[0], operands[1]))
>>  DONE;
>> @@ -9662,8 +9658,7 @@ (define_split
>>[(set (match_operand:DI 0 "int_reg_operand_not_pseudo")
>>  (match_operand:DI 1 "const_int_operand"))]
>>"TARGET_POWERPC64 && num_insns_constant (operands[1], DImode) > 1"
>> -  [(set (match_dup 0) (match_dup 2))
>> -   (set (match_dup 0) (plus:DI (match_dup 0) (match_dup 3)))]
>> +  [(pc)]
>>  {
>>if (rs6000_emit_set_const (operands[0], operands[1]))
>>  DONE;
>> @@ -9675,8 +9670,7 @@ (define_split
>>[(set (match_operand:DI 0 "int_reg_operand_not_pseudo")
>>  (match_operand:DI 1 "const_scalar_int_operand"))]
>>"TARGET_POWERPC64 && num_insns_constant (operands[1], DImode) > 1"
>> -  [(set (match_dup 0) (match_dup 2))
>> -   (set (match_dup 0) (plus:DI (match_dup 0) (match_dup 3)))]
>> +  [(pc)]
>>  {
>>if (rs6000_emit_set_const (operands[0], operands[1]))
>>  DONE;


Re: [PATCH] rs6000: avoid ineffective replacement of splitters

2022-08-12 Thread Kewen.Lin via Gcc-patches
Hi Jeff,

on 2022/8/12 14:39, Jiufu Guo via Gcc-patches wrote:
> Hi,
> 
> As a comment in
> https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599556.html
> 
> Those splitters call rs6000_emit_set_const directly, and the replacements
> are never used.  Using (pc) would be less misleading.

Since the replacements are never used, IMHO this subject doesn't
quite meet the change.  How about "fix misleading new patterns
of splitters"?

> 
> This patch pass bootstrap on ppc64 BE and LE.
> Is this ok for trunk.

This patch is OK w/ or w/o subject tweaked.  Thanks!

BR,
Kewen

> 
> BR,
> Jeff(Jiufu)
> 
> gcc/ChangeLog:
> 
>   * config/rs6000/rs6000.md: (constant splitters): Use "(pc)" as the
>   replacements.
> 
> ---
>  gcc/config/rs6000/rs6000.md | 12 +++-
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 1367a2cb779..7fadbeef1aa 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -7727,11 +7727,7 @@ (define_split
>[(set (match_operand:SI 0 "gpc_reg_operand")
>   (match_operand:SI 1 "const_int_operand"))]
>"num_insns_constant (operands[1], SImode) > 1"
> -  [(set (match_dup 0)
> - (match_dup 2))
> -   (set (match_dup 0)
> - (ior:SI (match_dup 0)
> - (match_dup 3)))]
> +  [(pc)]
>  {
>if (rs6000_emit_set_const (operands[0], operands[1]))
>  DONE;
> @@ -9662,8 +9658,7 @@ (define_split
>[(set (match_operand:DI 0 "int_reg_operand_not_pseudo")
>   (match_operand:DI 1 "const_int_operand"))]
>"TARGET_POWERPC64 && num_insns_constant (operands[1], DImode) > 1"
> -  [(set (match_dup 0) (match_dup 2))
> -   (set (match_dup 0) (plus:DI (match_dup 0) (match_dup 3)))]
> +  [(pc)]
>  {
>if (rs6000_emit_set_const (operands[0], operands[1]))
>  DONE;
> @@ -9675,8 +9670,7 @@ (define_split
>[(set (match_operand:DI 0 "int_reg_operand_not_pseudo")
>   (match_operand:DI 1 "const_scalar_int_operand"))]
>"TARGET_POWERPC64 && num_insns_constant (operands[1], DImode) > 1"
> -  [(set (match_dup 0) (match_dup 2))
> -   (set (match_dup 0) (plus:DI (match_dup 0) (match_dup 3)))]
> +  [(pc)]
>  {
>if (rs6000_emit_set_const (operands[0], operands[1]))
>  DONE;


[PATCH] rs6000: avoid ineffective replacement of splitters

2022-08-12 Thread Jiufu Guo via Gcc-patches
Hi,

As a comment in
https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599556.html

Those splitters call rs6000_emit_set_const directly, and the replacements
are never used.  Using (pc) would be less misleading.

This patch pass bootstrap on ppc64 BE and LE.
Is this ok for trunk.

BR,
Jeff(Jiufu)

gcc/ChangeLog:

* config/rs6000/rs6000.md: (constant splitters): Use "(pc)" as the
replacements.

---
 gcc/config/rs6000/rs6000.md | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 1367a2cb779..7fadbeef1aa 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -7727,11 +7727,7 @@ (define_split
   [(set (match_operand:SI 0 "gpc_reg_operand")
(match_operand:SI 1 "const_int_operand"))]
   "num_insns_constant (operands[1], SImode) > 1"
-  [(set (match_dup 0)
-   (match_dup 2))
-   (set (match_dup 0)
-   (ior:SI (match_dup 0)
-   (match_dup 3)))]
+  [(pc)]
 {
   if (rs6000_emit_set_const (operands[0], operands[1]))
 DONE;
@@ -9662,8 +9658,7 @@ (define_split
   [(set (match_operand:DI 0 "int_reg_operand_not_pseudo")
(match_operand:DI 1 "const_int_operand"))]
   "TARGET_POWERPC64 && num_insns_constant (operands[1], DImode) > 1"
-  [(set (match_dup 0) (match_dup 2))
-   (set (match_dup 0) (plus:DI (match_dup 0) (match_dup 3)))]
+  [(pc)]
 {
   if (rs6000_emit_set_const (operands[0], operands[1]))
 DONE;
@@ -9675,8 +9670,7 @@ (define_split
   [(set (match_operand:DI 0 "int_reg_operand_not_pseudo")
(match_operand:DI 1 "const_scalar_int_operand"))]
   "TARGET_POWERPC64 && num_insns_constant (operands[1], DImode) > 1"
-  [(set (match_dup 0) (match_dup 2))
-   (set (match_dup 0) (plus:DI (match_dup 0) (match_dup 3)))]
+  [(pc)]
 {
   if (rs6000_emit_set_const (operands[0], operands[1]))
 DONE;
-- 
2.17.1