[rs6000] Add two peephole2 patterns for mr. insn

  Hi,
  By checking the object files of SPECint, I found that two kinds of
compare/move can't be combined to "mr." pattern as there is no register
link between them. The patch adds two peephole2 patterns for them.

  Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.

Thanks
Gui Haochen



ChangeLog
rs6000: Add two peephole patterns for "mr." insn

Following two insns are never to be combined to "mr." pattern as there
is no register link between them.  So the patch adds these two peepholes.

cmp 0,3,0
mr 4,3

mr 4,3
cmp 0,3,0

gcc/
	* config/rs6000/rs6000.md (peephole2 for compare and move): New.
	(peephole2 for move and compare): New.

gcc/testsuite/
	* gcc.dg/rtl/powerpc/move_compare_peephole_32.c: New.
	* gcc.dg/rtl/powerpc/move_compare_peephole_64.c: New.

patch.diff
  

Hi Haochen,

on 2023/5/30 14:32, HAO CHEN GUI wrote:
> Hi,
>   By checking the object files of SPECint, I found that two kinds of
> compare/move can't be combined to "mr." pattern as there is no register
> link between them. The patch adds two peephole2 patterns for them.
> 

Thanks for improving this!

I know you had investigated Power specific move and set flag (mr.) for
something similar to what PR87871#c30 said, it seems good to also say
something about what triggered you to make this patch and test.  :)

>   Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
> 
> Thanks
> Gui Haochen
> 
> 
> 
> ChangeLog
> rs6000: Add two peephole patterns for "mr." insn
> 
> Following two insns are never to be combined to "mr." pattern as there
> is no register link between them.  So the patch adds these two peepholes.
> 
> cmp 0,3,0
> mr 4,3
> 
> mr 4,3
> cmp 0,3,0
> 
> gcc/
> 	* config/rs6000/rs6000.md (peephole2 for compare and move): New.
> 	(peephole2 for move and compare): New.
> 
> gcc/testsuite/
> 	* gcc.dg/rtl/powerpc/move_compare_peephole_32.c: New.
> 	* gcc.dg/rtl/powerpc/move_compare_peephole_64.c: New.
> 
> patch.diff
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index b0db8ae508d..b60230293f9 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -7891,6 +7891,36 @@ (define_insn "*mov<mode>_internal2"
>     (set_attr "dot" "yes")
>     (set_attr "length" "4,4,8")])
> 
> +(define_peephole2
> +  [(set (match_operand:CC 2 "cc_reg_operand" "")
> +	(compare:CC (match_operand:P 1 "int_reg_operand" "")
> +		    (const_int 0)))
> +   (set (match_operand:P 0 "int_reg_operand" "")
> +	(match_dup 1))]

The used mode_iterator P has the definition

(define_mode_iterator P [(SI "TARGET_32BIT") (DI "TARGET_64BIT")]),

it's not consistent with what you checked in test cases, which checks
has_arch_ppc64, -m32 -mpower64 can make has_arch_ppc64 satisfied but
the Pmode at that time is SImode, the case move_compare_peephole_64
will fail?

> +  "!cc_reg_not_cr0_operand (operands[2], CCmode)"
> +  [(parallel [(set (match_operand:CC 2 "cc_reg_operand" "=x")
> +		   (compare:CC (match_operand:P 1 "int_reg_operand" "r")
> +		   (const_int 0)))
> +	      (set (match_operand:P 0 "int_reg_operand" "=r")
> +		   (match_dup 1))])]
> +  ""
> +)
> +
> +(define_peephole2
> +  [(set (match_operand:P 0 "int_reg_operand" "")
> +	(match_operand:P 1 "int_reg_operand" ""))
> +   (set (match_operand:CC 2 "cc_reg_operand" "")
> +	(compare:CC (match_dup 1)
> +		    (const_int 0)))]
> +  "!cc_reg_not_cr0_operand (operands[2], CCmode)"
> +  [(parallel [(set (match_operand:CC 2 "cc_reg_operand" "=x")
> +		   (compare:CC (match_operand:P 1 "int_reg_operand" "r")
> +		   (const_int 0)))
> +	      (set (match_operand:P 0 "int_reg_operand" "=r")
> +		   (match_dup 1))])]
> +  ""
> +)
> +
>  (define_split
>    [(set (match_operand:CC 2 "cc_reg_not_cr0_operand")
>  	(compare:CC (match_operand:P 1 "gpc_reg_operand")
> diff --git a/gcc/testsuite/gcc.dg/rtl/powerpc/move_compare_peephole_32.c b/gcc/testsuite/gcc.dg/rtl/powerpc/move_compare_peephole_32.c
> new file mode 100644
> index 00000000000..4e094c8fe74
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/rtl/powerpc/move_compare_peephole_32.c
> @@ -0,0 +1,47 @@
> +/* { dg-do compile { target powerpc*-*-* } } */
> +/* { dg-skip-if "" { has_arch_ppc64 } } */
> +/* { dg-options "-O2 -mregnames" } */
> +

If the __RTL functions are extracted from functions of some source
files in test suite or typical external project, could you also
comment the related information in case one day someone would like
to check these sequences are still generated?

BR,
Kewen

> +int __RTL (startwith ("peephole2")) compare_move_peephole ()
> +{
> +(function "compare_move_peephole"
> +  (insn-chain
> +    (block 2
> +      (edge-from entry (flags "FALLTHRU"))
> +      (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
> +      (cinsn 8 (set (reg:CC %cr0)
> +                    (compare:CC (reg:SI %r3)
> +                        (const_int 0))))
> +      (cinsn 2 (set (reg:SI %r4)
> +                    (reg:SI %r3)))
> +      ;; Extra insn to avoid the above being deleted by DCE.
> +      (cinsn 18 (use (reg:SI %r4)))
> +      (cinsn 19 (use (reg:CC %cr0)))
> +      (edge-to exit (flags "FALLTHRU"))
> +    ) ;; block 2
> +  ) ;; insn-chain
> +) ;; function "main"
> +}
> +
> +int __RTL (startwith ("peephole2")) move_compare_peephole ()
> +{
> +(function "move_compare_peephole"
> +  (insn-chain
> +    (block 2
> +      (edge-from entry (flags "FALLTHRU"))
> +      (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
> +      (cinsn 2 (set (reg:SI %r4)
> +                    (reg:SI %r3)))
> +      (cinsn 8 (set (reg:CC %cr0)
> +                    (compare:CC (reg:SI %r3)
> +                        (const_int 0))))
> +      ;; Extra insn to avoid the above being deleted by DCE.
> +      (cinsn 18 (use (reg:SI %r4)))
> +      (cinsn 19 (use (reg:CC %cr0)))
> +      (edge-to exit (flags "FALLTHRU"))
> +    ) ;; block 2
> +  ) ;; insn-chain
> +) ;; function "main"
> +}
> +
> +/* { dg-final { scan-assembler-times {\mmr\.} 2 } } */
> diff --git a/gcc/testsuite/gcc.dg/rtl/powerpc/move_compare_peephole_64.c b/gcc/testsuite/gcc.dg/rtl/powerpc/move_compare_peephole_64.c
> new file mode 100644
> index 00000000000..511d6cc5317
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/rtl/powerpc/move_compare_peephole_64.c
> @@ -0,0 +1,47 @@
> +/* { dg-do compile { target powerpc*-*-* } } */
> +/* { dg-options "-O2 -mregnames" } */
> +/* { dg-require-effective-target has_arch_ppc64 } */
> +
> +int __RTL (startwith ("peephole2")) compare_move_peephole ()
> +{
> +(function "compare_move_peephole"
> +  (insn-chain
> +    (block 2
> +      (edge-from entry (flags "FALLTHRU"))
> +      (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
> +      (cinsn 8 (set (reg:CC %cr0)
> +                    (compare:CC (reg:DI %r3)
> +                        (const_int 0))))
> +      (cinsn 2 (set (reg:DI %r4)
> +                    (reg:DI %r3)))
> +      ;; Extra insn to avoid the above being deleted by DCE.
> +      (cinsn 18 (use (reg:DI %r4)))
> +      (cinsn 19 (use (reg:CC %cr0)))
> +      (edge-to exit (flags "FALLTHRU"))
> +    ) ;; block 2
> +  ) ;; insn-chain
> +) ;; function "main"
> +}
> +
> +int __RTL (startwith ("peephole2")) move_compare_peephole ()
> +{
> +(function "move_compare_peephole"
> +  (insn-chain
> +    (block 2
> +      (edge-from entry (flags "FALLTHRU"))
> +      (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
> +      (cinsn 2 (set (reg:DI %r4)
> +                    (reg:DI %r3)))
> +      (cinsn 8 (set (reg:CC %cr0)
> +                    (compare:CC (reg:DI %r3)
> +                        (const_int 0))))
> +      ;; Extra insn to avoid the above being deleted by DCE.
> +      (cinsn 18 (use (reg:DI %r4)))
> +      (cinsn 19 (use (reg:CC %cr0)))
> +      (edge-to exit (flags "FALLTHRU"))
> +    ) ;; block 2
> +  ) ;; insn-chain
> +) ;; function "main"
> +}
> +
> +/* { dg-final { scan-assembler-times {\mmr\.} 2 } } */
  

On Tue, 30 May 2023, HAO CHEN GUI via Gcc-patches wrote:

> +++ b/gcc/config/rs6000/rs6000.md
> @@ -7891,6 +7891,36 @@ (define_insn "*mov<mode>_internal2"
>     (set_attr "dot" "yes")
>     (set_attr "length" "4,4,8")])
> 
> +(define_peephole2
> +  [(set (match_operand:CC 2 "cc_reg_operand" "")
> +	(compare:CC (match_operand:P 1 "int_reg_operand" "")
> +		    (const_int 0)))
> +   (set (match_operand:P 0 "int_reg_operand" "")

A random comment from the sideline: I'd suggest to remove the 
(empty) constraints string from your peephole2's.

It can be a matter of port-specific-taste but it seems removing 
them would be consistent with the other peephole2's in 
rs6000.md.

(In this matter, I believe the examples in md.texi are bad.)

brgds, H-P
  

HP,
  It makes sense. I will update the patch.

Thanks
Gui Haochen

在 2023/6/20 8:07, Hans-Peter Nilsson 写道:
> On Tue, 30 May 2023, HAO CHEN GUI via Gcc-patches wrote:
> 
>> +++ b/gcc/config/rs6000/rs6000.md
>> @@ -7891,6 +7891,36 @@ (define_insn "*mov<mode>_internal2"
>>     (set_attr "dot" "yes")
>>     (set_attr "length" "4,4,8")])
>>
>> +(define_peephole2
>> +  [(set (match_operand:CC 2 "cc_reg_operand" "")
>> +	(compare:CC (match_operand:P 1 "int_reg_operand" "")
>> +		    (const_int 0)))
>> +   (set (match_operand:P 0 "int_reg_operand" "")
> 
> A random comment from the sideline: I'd suggest to remove the 
> (empty) constraints string from your peephole2's.
> 
> It can be a matter of port-specific-taste but it seems removing 
> them would be consistent with the other peephole2's in 
> rs6000.md.
> 
> (In this matter, I believe the examples in md.texi are bad.)
> 
> brgds, H-P