Re: [PATCH][AArch64] Peephole for SUBS

[James Greenhalgh <james.greenhalgh@arm.com>]

On Fri, Apr 21, 2017 at 09:20:52AM +0100, Kyrill Tkachov wrote:
> Hi all,
> 
> A pattern that sometimes occurs in the wild is to subtract two operands and
> separately compare them. This can be implemented as a single SUBS instruction
> and we actually have a define_insn for this: sub<mode>3_compare1.  However,
> I'm not sure if that's enough by itself to match these constructs. Adding a
> peephole that will actually bring the subtraction and comparison SETs
> together into a PARALLEL helps a lot in matching these (note that there is no
> dependency between the subtract and the comparison).
> 
> This patch adds such a peephole. It's really simple and straightforward. The
> only thing to look out for is the case when the output of the subtract is a
> register that is also one of the operands:
> SUB W0, W0, W1
> CMP W0, W1
> 
> should not be transformed into:
> SUBS W0, W0, W1.
> 
> The testcase in the patch provides a motivating example where we now generate
> a single SUBS instead of a SUB followed by a CMP.
> 
> This transformation triggers a few times in SPEC2006. Not enough to actually
> move the needle, but it's the Right Thing to Do (tm).
> 
> I've seen it catch cases that compute an absolute difference, for example:
> int
> foo (int a, int b)
> {
>   if (a < b)
>     return b - a;
>   else
>     return a - b;
> }
> 
> will now generate:
> foo:
>         sub     w2, w1, w0
>         subs    w3, w0, w1
>         csel    w0, w3, w2, ge
>         ret
> 
> instead of:
> foo:
>         sub     w2, w1, w0
>         sub     w3, w0, w1
>         cmp     w0, w1
>         csel    w0, w3, w2, ge
>         ret
> 
> 
> Bootstrapped and tested on aarch64-none-linux-gnu.
> 
> Ok for GCC 8?

OK.

Thanks,
James

> 
> Thanks,
> Kyrill
> 
> 2017-04-21  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>     * config/aarch64/aarch64.c (define_peephole2 above
>     *sub_<shift>_<mode>): New peephole.
> 
> 2017-04-21  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>     * gcc.target/aarch64/subs_compare_1.c: New test.