diff mbox series

[x86] PR target/47949: Use xchg to move from/to AX_REG with -Oz.

Message ID 004a01d8a680$6f87dff0$4e979fd0$@nextmovesoftware.com
State New, archived
Headers
Series [x86] PR target/47949: Use xchg to move from/to AX_REG with -Oz. |

Commit Message

Roger Sayle Aug. 2, 2022, 2:59 p.m. UTC 
  This patch adds a peephole2 to i386.md to implement the suggestion in
PR target/47949, of using xchg instead of mov for moving values to/from
the %rax/%eax register, controlled by -Oz, as the xchg instruction is
one byte shorter than the move it is replacing.

The new test case is taken from the PR:
int foo(int x) { return x; }

where previously we'd generate:
foo:    mov %edi,%eax  // 2 bytes
        ret

but with this patch, using -Oz, we generate:
foo:    xchg %eax,%edi  // 1 byte
        ret

On the CSiBE benchmark, this saves a total of 10238 bytes (reducing
the -Oz total from 3661796 bytes to 3651558 bytes, a 0.28% saving).

Interestingly, some modern architectures (such as Zen 3) implement
xchg using zero latency register renaming (just like mov), so in theory
this transformation could be enabled when optimizing for speed, if
benchmarking shows the improved code density produces consistently
better performance.  However, this is architecture dependent, and
there may be interactions using xchg (instead a single_set) in the
late RTL passes (such as cprop_hardreg), so for now I've restricted
this to -Oz.

This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
and make -k check, both with and without --target_board=unix{-m32},
with no new failures.  Ok for mainline?


2022-08-02  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
        PR target/47949
        * config/i386/i386.md (peephole2): New peephole2 to convert
        SWI48 moves to/from %rax/%eax where the src is dead to xchg,
        when optimizing for minimal size with -Oz.

gcc/testsuite/ChangeLog
        PR target/47949
        * gcc.target/i386/pr47949.c: New test case.


Thanks,
Roger
--

Comments

Uros Bizjak Aug. 2, 2022, 5:33 p.m. UTC | #1 
On Tue, Aug 2, 2022 at 4:59 PM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> This patch adds a peephole2 to i386.md to implement the suggestion in
> PR target/47949, of using xchg instead of mov for moving values to/from
> the %rax/%eax register, controlled by -Oz, as the xchg instruction is
> one byte shorter than the move it is replacing.
>
> The new test case is taken from the PR:
> int foo(int x) { return x; }
>
> where previously we'd generate:
> foo:    mov %edi,%eax  // 2 bytes
>         ret
>
> but with this patch, using -Oz, we generate:
> foo:    xchg %eax,%edi  // 1 byte
>         ret
>
> On the CSiBE benchmark, this saves a total of 10238 bytes (reducing
> the -Oz total from 3661796 bytes to 3651558 bytes, a 0.28% saving).
>
> Interestingly, some modern architectures (such as Zen 3) implement
> xchg using zero latency register renaming (just like mov), so in theory
> this transformation could be enabled when optimizing for speed, if
> benchmarking shows the improved code density produces consistently
> better performance.  However, this is architecture dependent, and
> there may be interactions using xchg (instead a single_set) in the
> late RTL passes (such as cprop_hardreg), so for now I've restricted
> this to -Oz.
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> and make -k check, both with and without --target_board=unix{-m32},
> with no new failures.  Ok for mainline?
>
>
> 2022-08-02  Roger Sayle  <roger@nextmovesoftware.com>
>
> gcc/ChangeLog
>         PR target/47949
>         * config/i386/i386.md (peephole2): New peephole2 to convert
>         SWI48 moves to/from %rax/%eax where the src is dead to xchg,
>         when optimizing for minimal size with -Oz.
>
> gcc/testsuite/ChangeLog
>         PR target/47949
>         * gcc.target/i386/pr47949.c: New test case.

OK.

BTW: The testcase can be improved to cover Dimode as well as SImode case:

+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-Oz" } */
+
+int foo(int x)

long foo (long x, long y)
{
   return y;
}

And add additional options of -mregparm=2 for ia32. This will move
%edx to %eax for 32bit and %rsi to %rax for 64bit targets.

Thanks,
Uros.
diff mbox series

Patch

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index f1158e1..11629ce 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -3018,6 +3018,18 @@ 
   [(parallel [(set (match_dup 1) (match_dup 2))
 	      (set (match_dup 2) (match_dup 1))])])
 
+;; Convert moves to/from AX_REG into xchg with -Oz.
+(define_peephole2
+  [(set (match_operand:SWI48 0 "general_reg_operand")
+	(match_operand:SWI48 1 "general_reg_operand"))]
+ "optimize_size > 1
+  && (REGNO (operands[0]) == AX_REG
+      || REGNO (operands[1]) == AX_REG)
+  && optimize_insn_for_size_p ()
+  && peep2_reg_dead_p (1, operands[1])"
+  [(parallel [(set (match_dup 0) (match_dup 1))
+	      (set (match_dup 1) (match_dup 0))])])
+
 (define_expand "movstrict<mode>"
   [(set (strict_low_part (match_operand:SWI12 0 "register_operand"))
 	(match_operand:SWI12 1 "general_operand"))]
diff --git a/gcc/testsuite/gcc.target/i386/pr47949.c b/gcc/testsuite/gcc.target/i386/pr47949.c
new file mode 100644
index 0000000..7a58fa4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr47949.c
@@ -0,0 +1,9 @@ 
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-Oz" } */
+
+int foo(int x)
+{
+  return x;
+}
+
+/* { dg-final { scan-assembler "xchg" } } */