* [PATCH][ARM] minmax_arithsi for non-canonical operand order with MINUS operator
@ 2013-03-21 18:09 Kyrylo Tkachov
0 siblings, 0 replies; 4+ messages in thread
From: Kyrylo Tkachov @ 2013-03-21 18:09 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Earnshaw, Ramana Radhakrishnan
[-- Attachment #1: Type: text/plain, Size: 834 bytes --]
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?
Thanks,
Kyrill
gcc/
2013-03-21 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
* config/arm/arm.md (minmax_arithsi_non_canon): New pattern.
gcc/testsuite
2013-03-21 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
* gcc.target/arm/minmax_minus.c: New test.
[-- Attachment #2: minmax_arithsi.txt --]
[-- Type: text/plain, Size: 2620 bytes --]
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index f3c59f3..c2875f9 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -3423,6 +3423,49 @@
(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])"
+ [(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, SImode,
+ 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" } } */
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][ARM] minmax_arithsi for non-canonical operand order with MINUS operator
2013-04-02 17:20 ` Kyrylo Tkachov
@ 2013-04-09 17:52 ` Richard Earnshaw
0 siblings, 0 replies; 4+ messages in thread
From: Richard Earnshaw @ 2013-04-09 17:52 UTC (permalink / raw)
To: Kyrylo Tkachov; +Cc: Ramana Radhakrishnan, gcc-patches
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH][ARM] minmax_arithsi for non-canonical operand order with MINUS operator
2013-04-02 10:51 ` Ramana Radhakrishnan
@ 2013-04-02 17:20 ` Kyrylo Tkachov
2013-04-09 17:52 ` Richard Earnshaw
0 siblings, 1 reply; 4+ messages in thread
From: Kyrylo Tkachov @ 2013-04-02 17:20 UTC (permalink / raw)
To: Ramana Radhakrishnan; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1902 bytes --]
> 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 #2: minmax_arithsi.txt --]
[-- Type: text/plain, Size: 2645 bytes --]
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" } } */
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][ARM] minmax_arithsi for non-canonical operand order with MINUS operator
[not found] <514b4cf7.c9c4440a.7a38.789dSMTPIN_ADDED_BROKEN@mx.google.com>
@ 2013-04-02 10:51 ` Ramana Radhakrishnan
2013-04-02 17:20 ` Kyrylo Tkachov
0 siblings, 1 reply; 4+ messages in thread
From: Ramana Radhakrishnan @ 2013-04-02 10:51 UTC (permalink / raw)
To: Kyrylo Tkachov; +Cc: gcc-patches, Richard Earnshaw, Ramana Radhakrishnan
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/ .
regards
Ramana
> Thanks,
> Kyrill
>
> gcc/
> 2013-03-21 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>
> * config/arm/arm.md (minmax_arithsi_non_canon): New pattern.
>
> gcc/testsuite
> 2013-03-21 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>
> * gcc.target/arm/minmax_minus.c: New test.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-04-09 15:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-21 18:09 [PATCH][ARM] minmax_arithsi for non-canonical operand order with MINUS operator Kyrylo Tkachov
[not found] <514b4cf7.c9c4440a.7a38.789dSMTPIN_ADDED_BROKEN@mx.google.com>
2013-04-02 10:51 ` Ramana Radhakrishnan
2013-04-02 17:20 ` Kyrylo Tkachov
2013-04-09 17:52 ` Richard Earnshaw
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).