| Message ID | CA+=Sn1==yeUP+2nZ_M2Cfva8qaD553wDPer85axLuN+Q_OZAzg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 03/01/13 22:39, Andrew Pinski wrote: > Hi, > For aarch64, we don't CSE some cmp away. This patch fixes the case > where we are CSE across some basic-blocks like: > int f(int a, int b) > { > if(a<b) > return 1; > if(a>b) > return -1; > return 0; > } > --- CUT --- > To fix this, I implemented the target hook > TARGET_FIXED_CONDITION_CODE_REGS as there was already code in CSE > which uses this target hook to find the extra setting of the CC > registers. > > OK? Build and tested on aarch64-thunder-elf (using Cavium's internal > simulator). Build a cross to aarch64-thunder-linux-gnu and a Canadian > cross with that one for the native toolchain. > > Thanks, > Andrew Pinski > > * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs): > New function. > (TARGET_FIXED_CONDITION_CODE_REGS): Define. > > * gcc.target/aarch64/cmp-1.c: New testcase. > > > fixed_condition_code.diff.txt > > > * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs): > New function. > (TARGET_FIXED_CONDITION_CODE_REGS): Define. > > * gcc.target/aarch64/cmp-1.c: New testcase. > Index: config/aarch64/aarch64.c > =================================================================== > --- config/aarch64/aarch64.c (revision 194872) > +++ config/aarch64/aarch64.c (working copy) > @@ -3041,6 +3041,16 @@ aarch64_const_double_zero_rtx_p (rtx x) > return REAL_VALUES_EQUAL (r, dconst0); > } > > +/* Return the fixed registers used for condition codes. */ > + > +static bool > +aarch64_fixed_condition_code_regs (unsigned int *p1, unsigned int *p2) > +{ > + *p1 = CC_REGNUM; > + *p2 = -1; Please use INVALID_REGNUM here (as the documentation states). Otherwise, OK. A backport to the AArch64-4.7 branch would be appreciated. R.
On Fri, Jan 4, 2013 at 1:34 AM, Richard Earnshaw <rearnsha@arm.com> wrote: > On 03/01/13 22:39, Andrew Pinski wrote: >> >> Hi, >> For aarch64, we don't CSE some cmp away. This patch fixes the case >> where we are CSE across some basic-blocks like: >> int f(int a, int b) >> { >> if(a<b) >> return 1; >> if(a>b) >> return -1; >> return 0; >> } >> --- CUT --- >> To fix this, I implemented the target hook >> TARGET_FIXED_CONDITION_CODE_REGS as there was already code in CSE >> which uses this target hook to find the extra setting of the CC >> registers. >> >> OK? Build and tested on aarch64-thunder-elf (using Cavium's internal >> simulator). Build a cross to aarch64-thunder-linux-gnu and a Canadian >> cross with that one for the native toolchain. >> >> Thanks, >> Andrew Pinski >> >> * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs): >> New function. >> (TARGET_FIXED_CONDITION_CODE_REGS): Define. >> >> * gcc.target/aarch64/cmp-1.c: New testcase. >> >> >> fixed_condition_code.diff.txt >> >> >> >> * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs): >> New function. >> (TARGET_FIXED_CONDITION_CODE_REGS): Define. >> >> * gcc.target/aarch64/cmp-1.c: New testcase. > > >> Index: config/aarch64/aarch64.c >> =================================================================== >> --- config/aarch64/aarch64.c (revision 194872) >> +++ config/aarch64/aarch64.c (working copy) >> @@ -3041,6 +3041,16 @@ aarch64_const_double_zero_rtx_p (rtx x) >> return REAL_VALUES_EQUAL (r, dconst0); >> } >> >> +/* Return the fixed registers used for condition codes. */ >> + >> +static bool >> +aarch64_fixed_condition_code_regs (unsigned int *p1, unsigned int *p2) >> +{ >> + *p1 = CC_REGNUM; >> + *p2 = -1; > > > Please use INVALID_REGNUM here (as the documentation states). The comment in target.def says: Up to two condition code registers are supported. If there is only one for this target, the int pointed at by the second argument should be set to -1. */ While tm.texi says: arguments point to the hard register numbers used for condition codes. When there is only one such register, as is true on most systems, the integer pointed to by @var{p2} should be set to @code{INVALID_REGNUM}. I had just read the comment in target.def when I was writing this patch which is why I had used -1. I agree INVALID_REGNUM is better. I will send out a patch to fix the comment in target.def later. > Otherwise, OK. > > A backport to the AArch64-4.7 branch would be appreciated. I don't have time to do a backport and to test it. Thanks, Andrew Pinski
Index: testsuite/gcc.target/aarch64/cmp-1.c =================================================================== --- testsuite/gcc.target/aarch64/cmp-1.c (revision 0) +++ testsuite/gcc.target/aarch64/cmp-1.c (revision 0) @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +int f(int a, int b) +{ + if(a<b) + return 1; + if(a>b) + return -1; + return 0; +} + +/* We should optimize away the second cmp. */ +/* { dg-final { scan-assembler-times "cmp\tw" 1 } } */ + Index: config/aarch64/aarch64.c =================================================================== --- config/aarch64/aarch64.c (revision 194872) +++ config/aarch64/aarch64.c (working copy) @@ -3041,6 +3041,16 @@ aarch64_const_double_zero_rtx_p (rtx x) return REAL_VALUES_EQUAL (r, dconst0); } +/* Return the fixed registers used for condition codes. */ + +static bool +aarch64_fixed_condition_code_regs (unsigned int *p1, unsigned int *p2) +{ + *p1 = CC_REGNUM; + *p2 = -1; + return true; +} + enum machine_mode aarch64_select_cc_mode (RTX_CODE code, rtx x, rtx y) { @@ -7551,6 +7561,9 @@ aarch64_vectorize_vec_perm_const_ok (enu #define TARGET_VECTORIZE_VEC_PERM_CONST_OK \ aarch64_vectorize_vec_perm_const_ok + +#define TARGET_FIXED_CONDITION_CODE_REGS aarch64_fixed_condition_code_regs + struct gcc_target targetm = TARGET_INITIALIZER; #include "gt-aarch64.h"
Hi, For aarch64, we don't CSE some cmp away. This patch fixes the case where we are CSE across some basic-blocks like: int f(int a, int b) { if(a<b) return 1; if(a>b) return -1; return 0; } --- CUT --- To fix this, I implemented the target hook TARGET_FIXED_CONDITION_CODE_REGS as there was already code in CSE which uses this target hook to find the extra setting of the CC registers. OK? Build and tested on aarch64-thunder-elf (using Cavium's internal simulator). Build a cross to aarch64-thunder-linux-gnu and a Canadian cross with that one for the native toolchain. Thanks, Andrew Pinski * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs): New function. (TARGET_FIXED_CONDITION_CODE_REGS): Define. * gcc.target/aarch64/cmp-1.c: New testcase. * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs): New function. (TARGET_FIXED_CONDITION_CODE_REGS): Define. * gcc.target/aarch64/cmp-1.c: New testcase.