RE: [PATCH][ARM] minmax_arithsi for non-canonical operand order with MINUS operator

["Kyrylo Tkachov" <kyrylo dot tkachov at arm dot com>]

> From: Ramana Radhakrishnan [mailto:ramana.gcc@googlemail.com]
> Sent: 02 April 2013 11:10
> To: Kyrylo Tkachov
> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw; Ramana Radhakrishnan
> Subject: Re: [PATCH][ARM] minmax_arithsi for non-canonical operand
> order with MINUS operator
> 
> On Thu, Mar 21, 2013 at 6:09 PM, Kyrylo Tkachov
> <kyrylo.tkachov@arm.com> wrote:
> > Hi all,
> >
> > This patch adds a splitter variant of the minmax_arithsi pattern for
> when
> > the operator
> > is non-commutative (MINUS) and the ordering of the operands is not
> > canonical.
> >
> > That is, it will trigger for:
> > #define MAX(a, b) (a > b ? a : b)
> > int
> > foo (int a, int b, int c)
> > {
> >   return c - MAX (a,b);
> > }
> >
> > and will generate:
> >          cmp     r1, r0
> >          rsbge   r0, r1, r2
> >          rsblt   r0, r0, r2
> >
> > instead of the current:
> >         cmp     r0, r1
> >         movlt   r0, r1
> >         rsb     r0, r0, r2
> >
> > No regressions on arm-none-eabi.
> >
> > Ok for trunk?
> >
> 
> Split after reload please into cond-exec or use if_then_else instead
> if you are splitting before reload - I originally thought it to be
> safe when you asked me, but then have gone back and corrected myself.
> 
> Read this thread . http://patches.linaro.org/6469/ .

Hi Ramana, thanks for the review.
How about this? We split after reload now.
Using if_then_else got me a lot of unrecognisable instruction ICEs and
delaying the split till after reload seemed like a cleaner approach.

Tested arm-none-eabi on qemu.


gcc/
2013-04-02  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

        * config/arm/arm.md (minmax_arithsi_non_canon): New pattern.

gcc/testsuite
2013-04-02  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

        * gcc.target/arm/minmax_minus.c: New test.


> regards
> Ramana
> 
Thanks,
Kyrill

Attachment: minmax_arithsi.txt
Description: Text document