| Message ID | 55B5650B.2060601@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Michael, It looks like this patch introduces regressions on armeb in: gcc.dg/vect/vect-reduc-7.c gcc.dg/vect/vect-reduc-8.c gcc.dg/vect/vect-reduc-9.c See http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/226476/report-build-info.html for a bit more details. Can you have a look? Thanks, Christophe. On 27 July 2015 at 10:17, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote: > Hi Michael, > > On 26/07/15 23:54, Michael Collison wrote: >> >> Here is an updated patch that addresses the issues you mentioned: >> >> 2015-07-24 Michael Collison <michael.collison@linaro.org >> Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> >> >> * gcc/config/arm/arm.md (*arm_smin_cmp): New pattern. >> (*arm_umin_cmp): Likewise. >> * gcc.target/arm/mincmp.c: Test min compare idiom. > > > Just "New test." would be a better ChangeLog entry for this. > > >> >> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md >> index 0be70a8..361c292 100644 >> --- a/gcc/config/arm/arm.md >> +++ b/gcc/config/arm/arm.md >> @@ -3455,6 +3455,44 @@ >> (set_attr "type" "multiple,multiple")] >> ) >> >> +;; t = (s/u)min (x, y) >> +;; cc = cmp (t, z) >> +;; is the same as >> +;; cmp x, z >> +;; cmpge(u) y, z >> + >> +(define_insn_and_split "*arm_smin_cmp" >> + [(set (reg:CC CC_REGNUM) >> + (compare:CC >> + (smin:SI (match_operand:SI 0 "s_register_operand" "r") >> + (match_operand:SI 1 "s_register_operand" "r")) >> + (match_operand:SI 2 "s_register_operand" "r")))] >> + "TARGET_32BIT" >> + "#" >> + "&& reload_completed" >> + [(set (reg:CC CC_REGNUM) >> + (compare:CC (match_dup 0) (match_dup 2))) >> + (cond_exec (ge:CC (reg:CC CC_REGNUM) (const_int 0)) >> + (set (reg:CC CC_REGNUM) >> + (compare:CC (match_dup 1) (match_dup 2))))] >> +) >> + >> +(define_insn_and_split "*arm_umin_cmp" >> + [(set (reg:CC CC_REGNUM) >> + (compare:CC >> + (umin:SI (match_operand:SI 0 "s_register_operand" "r") >> + (match_operand:SI 1 "s_register_operand" "r")) >> + (match_operand:SI 2 "s_register_operand" "r")))] >> + "TARGET_32BIT" >> + "#" >> + "&& reload_completed" >> + [(set (reg:CC CC_REGNUM) >> + (compare:CC (match_dup 0) (match_dup 2))) >> + (cond_exec (geu:CC (reg:CC CC_REGNUM) (const_int 0)) >> + (set (reg:CC CC_REGNUM) >> + (compare:CC (match_dup 1) (match_dup 2))))] >> +) >> + >> (define_expand "umaxsi3" >> [(parallel [ >> (set (match_operand:SI 0 "s_register_operand" "") >> diff --git a/gcc/testsuite/gcc.target/arm/mincmp.c >> b/gcc/testsuite/gcc.target/arm/mincmp.c >> new file mode 100644 >> index 0000000..2a55c6d >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/arm/mincmp.c >> @@ -0,0 +1,18 @@ >> +/* { dg-do compile } */ >> +/* { dg-options "-O2" } */ >> +/* { dg-require-effective-target arm32 } */ >> + >> +#define min(x, y) ((x) <= (y)) ? (x) : (y) >> + >> +unsigned int foo (unsigned int i, unsigned int x ,unsigned int y) > > > Comma after "unsigned int x" in the wrong place. > >> +{ >> + return i < (min (x, y)); >> +} >> + >> +int bar (int i, int x, int y) >> +{ >> + return i < (min (x, y)); >> +} > > > Can you please use GNU style for the testcase. > New line after return type, function name an arguments on their own line. > > Ok with these changes. > > Thanks, > Kyrill >
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 0be70a8..361c292 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -3455,6 +3455,44 @@ (set_attr "type" "multiple,multiple")] ) +;; t = (s/u)min (x, y) +;; cc = cmp (t, z) +;; is the same as +;; cmp x, z +;; cmpge(u) y, z + +(define_insn_and_split "*arm_smin_cmp" + [(set (reg:CC CC_REGNUM) + (compare:CC + (smin:SI (match_operand:SI 0 "s_register_operand" "r") + (match_operand:SI 1 "s_register_operand" "r")) + (match_operand:SI 2 "s_register_operand" "r")))] + "TARGET_32BIT" + "#" + "&& reload_completed" + [(set (reg:CC CC_REGNUM) + (compare:CC (match_dup 0) (match_dup 2))) + (cond_exec (ge:CC (reg:CC CC_REGNUM) (const_int 0)) + (set (reg:CC CC_REGNUM) + (compare:CC (match_dup 1) (match_dup 2))))] +) + +(define_insn_and_split "*arm_umin_cmp" + [(set (reg:CC CC_REGNUM) + (compare:CC + (umin:SI (match_operand:SI 0 "s_register_operand" "r") + (match_operand:SI 1 "s_register_operand" "r")) + (match_operand:SI 2 "s_register_operand" "r")))] + "TARGET_32BIT" + "#" + "&& reload_completed" + [(set (reg:CC CC_REGNUM) + (compare:CC (match_dup 0) (match_dup 2))) + (cond_exec (geu:CC (reg:CC CC_REGNUM) (const_int 0)) + (set (reg:CC CC_REGNUM) + (compare:CC (match_dup 1) (match_dup 2))))] +) + (define_expand "umaxsi3" [(parallel [ (set (match_operand:SI 0 "s_register_operand" "") diff --git a/gcc/testsuite/gcc.target/arm/mincmp.c b/gcc/testsuite/gcc.target/arm/mincmp.c new file mode 100644 index 0000000..2a55c6d --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/mincmp.c @@ -0,0 +1,18 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ +/* { dg-require-effective-target arm32 } */ + +#define min(x, y) ((x) <= (y)) ? (x) : (y) + +unsigned int foo (unsigned int i, unsigned int x ,unsigned int y) +{ + return i < (min (x, y)); +} + +int bar (int i, int x, int y) +{ + return i < (min (x, y)); +} + +/* { dg-final { scan-assembler "cmpcs" } } */ +/* { dg-final { scan-assembler "cmpge" } } */
Here is an updated patch that addresses the issues you mentioned: 2015-07-24 Michael Collison <michael.collison@linaro.org Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> * gcc/config/arm/arm.md (*arm_smin_cmp): New pattern. (*arm_umin_cmp): Likewise. * gcc.target/arm/mincmp.c: Test min compare idiom.