Message ID | 019501ce2fbc$00ea3830$02bea890$@tkachov@arm.com |
---|---|
State | New |
Headers | show |
On 02/04/13 17:06, Kyrylo Tkachov wrote: >> 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. > > OK. R.
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 8895080..7ff066e 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -3417,6 +3417,50 @@ (const_int 12)))] ) +; Reject the frame pointer in operand[1], since reloading this after +; it has been eliminated can cause carnage. +(define_insn_and_split "*minmax_arithsi_non_canon" + [(set (match_operand:SI 0 "s_register_operand" "=r,r") + (minus:SI + (match_operand:SI 1 "s_register_operand" "0,?r") + (match_operator:SI 4 "minmax_operator" + [(match_operand:SI 2 "s_register_operand" "r,r") + (match_operand:SI 3 "arm_rhs_operand" "rI,rI")]))) + (clobber (reg:CC CC_REGNUM))] + "TARGET_32BIT && !arm_eliminable_register (operands[1])" + "#" + "TARGET_32BIT && !arm_eliminable_register (operands[1]) && reload_completed" + [(set (reg:CC CC_REGNUM) + (compare:CC (match_dup 2) (match_dup 3))) + + (cond_exec (match_op_dup 4 [(reg:CC CC_REGNUM) (const_int 0)]) + (set (match_dup 0) + (minus:SI (match_dup 1) + (match_dup 2)))) + (cond_exec (match_op_dup 5 [(reg:CC CC_REGNUM) (const_int 0)]) + (set (match_dup 0) + (minus:SI (match_dup 1) + (match_dup 3))))] + { + enum machine_mode mode = SELECT_CC_MODE (GET_CODE (operands[1]), + operands[2], operands[3]); + enum rtx_code rc = minmax_code (operands[4]); + operands[4] = gen_rtx_fmt_ee (rc, VOIDmode, + operands[2], operands[3]); + + if (mode == CCFPmode || mode == CCFPEmode) + rc = reverse_condition_maybe_unordered (rc); + else + rc = reverse_condition (rc); + operands[5] = gen_rtx_fmt_ee (rc, SImode, operands[2], operands[3]); + } + [(set_attr "conds" "clob") + (set (attr "length") + (if_then_else (eq_attr "is_thumb" "yes") + (const_int 14) + (const_int 12)))] +) + (define_code_iterator SAT [smin smax]) (define_code_iterator SATrev [smin smax]) (define_code_attr SATlo [(smin "1") (smax "2")]) diff --git a/gcc/testsuite/gcc.target/arm/minmax_minus.c b/gcc/testsuite/gcc.target/arm/minmax_minus.c new file mode 100644 index 0000000..050c847 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/minmax_minus.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +#define MAX(a, b) (a > b ? a : b) +int +foo (int a, int b, int c) +{ + return c - MAX (a, b); +} + +/* { dg-final { scan-assembler "rsbge" } } */ +/* { dg-final { scan-assembler "rsblt" } } */