Hi Haochen,
on 2023/6/12 10:34, HAO CHEN GUI wrote:
> Hi,
> This patch adds two peephole2 patterns which help convert certain insn
> sequences to "mr." instruction. These insn sequences can't be combined in
> combine pass.
>
> Compared to last version, it adds a new mode iterator "Q" which should
> be used for dot instruction. With "-m32/-mpowerpc64" set, the dot
> instruction should compare DImode with 0, not the SImode.
>
> Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
>
> Thanks
> Gui Haochen
>
>
> ChangeLog
> rs6000: Add two peephole patterns for "mr." insn
>
> When investigating the issue mentioned in PR87871#c30 - if compare
> and move pattern benefits before RA, I checked the assembly generated
> for SPEC2017 and found that certain insn sequences aren't converted to
> "mr." instructions.
> Following two sequence are never to be combined to "mr." pattern as
> there is no register link between them. This patch adds two peephole2
> patterns to convert them to "mr." instructions.
>
> cmp 0,3,0
> mr 4,3
>
> mr 4,3
> cmp 0,3,0
>
> The patch also creates a new mode iterator which decided by
> TARGET_POWERPC64. This mode iterator is used in "mr." and its split
> pattern. The original P iterator is wrong when -m32/-mpowerpc64 is set.
> In this situation, the "mr." should compares the whole 64-bit register
> with 0 other than the low 32-bit one.
>
> gcc/
> * config/rs6000/rs6000.md (peephole2 for compare_and_move): New.
> (peephole2 for move_and_compare): New.
> (mode_iterator Q): New. Set the mode to SI/DImode by
> TARGET_POWERPC64.
> (*mov<mode>_internal2): Change the mode iterator from P to Q.
> (split pattern for compare_and_move): Likewise.
>
> 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..fdb5b6ed22a 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -491,6 +491,7 @@ (define_mode_iterator SDI [SI DI])
> ; The size of a pointer. Also, the size of the value that a record-condition
> ; (one with a '.') will compare; and the size used for arithmetic carries.
> (define_mode_iterator P [(SI "TARGET_32BIT") (DI "TARGET_64BIT")])
> +(define_mode_iterator Q [(SI "!TARGET_POWERPC64") (DI "TARGET_POWERPC64")])
The P mode iterator performs like Pmode, so it's named as P, but here the
proposed Q looks random? It's a bad name IMHO, since you want this mode
iterator to perform like word_mode, I'd suggest WORD instead of Q.
>
> ; Iterator to add PTImode along with TImode (TImode can go in VSX registers,
> ; PTImode is GPR only)
> @@ -7879,9 +7880,9 @@ (define_split
>
> (define_insn "*mov<mode>_internal2"
> [(set (match_operand:CC 2 "cc_reg_operand" "=y,x,?y")
> - (compare:CC (match_operand:P 1 "gpc_reg_operand" "0,r,r")
> + (compare:CC (match_operand:Q 1 "gpc_reg_operand" "0,r,r")
> (const_int 0)))
> - (set (match_operand:P 0 "gpc_reg_operand" "=r,r,r") (match_dup 1))]
> + (set (match_operand:Q 0 "gpc_reg_operand" "=r,r,r") (match_dup 1))]
> ""
> "@
> cmp<wd>i %2,%0,0
> @@ -7891,11 +7892,41 @@ (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:Q 1 "int_reg_operand" "")
> + (const_int 0)))
> + (set (match_operand:Q 0 "int_reg_operand" "")
> + (match_dup 1))]
> + "!cc_reg_not_cr0_operand (operands[2], CCmode)"
> + [(parallel [(set (match_operand:CC 2 "cc_reg_operand" "=x")
> + (compare:CC (match_operand:Q 1 "int_reg_operand" "r")
> + (const_int 0)))
> + (set (match_operand:Q 0 "int_reg_operand" "=r")
> + (match_dup 1))])]
> + ""
> +)
> +
> +(define_peephole2
> + [(set (match_operand:Q 0 "int_reg_operand" "")
> + (match_operand:Q 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:GPR 1 "int_reg_operand" "r")
> + (const_int 0)))
> + (set (match_operand:Q 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")
> + (compare:CC (match_operand:Q 1 "gpc_reg_operand")
> (const_int 0)))
> - (set (match_operand:P 0 "gpc_reg_operand") (match_dup 1))]
> + (set (match_operand:Q 0 "gpc_reg_operand") (match_dup 1))]
> "reload_completed"
> [(set (match_dup 0) (match_dup 1))
> (set (match_dup 2)
> 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..29234dea7c7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/rtl/powerpc/move_compare_peephole_32.c
> @@ -0,0 +1,60 @@
> +/* { dg-do compile { target powerpc*-*-* } } */
> +/* { dg-skip-if "" { has_arch_ppc64 } } */
> +/* { dg-options "-O2 -mregnames" } */
> +
> +/* Following instruction sequence is found in assembly of
> + Perl_block_start, which is a function of op.c in SPEC2017
> + perlbench. It can be never combined to a move and compare
> + instruction in combine pass. A peephole pattern is needed to
> + converted the sequence to a "mr." instruction.
> +
> + cmpdi 0,9,0
> + mr 12,9
> +
> + This test case is an analogue of the source code and verifies
> + if the peephole2 patterns work.
> +*/
Thanks for adding this. The others look good to me!
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..dd360033dbd
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/rtl/powerpc/move_compare_peephole_64.c
> @@ -0,0 +1,60 @@
> +/* { dg-do compile { target powerpc*-*-* } } */
> +/* { dg-options "-O2 -mregnames" } */
> +/* { dg-require-effective-target has_arch_ppc64 } */
> +
> +/* Following instruction sequence is found in assembly of
> + Perl_block_start, which is a function of op.c in SPEC2017
> + perlbench. It can be never combined to a move and compare
> + instruction in combine pass. A peephole pattern is needed to
> + converted the sequence to a "mr." instruction.
> +
> + cmpdi 0,9,0
> + mr 12,9
> +
> + This test case is an analogue of the source code and verifies
> + if the peephole2 patterns work.
> +*/
> +
> +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 } } */
