Message ID | 000001cf3cf6$a3ef8b00$ebcea100$@arm.com |
---|---|
State | New |
Headers | show |
On Tue, Mar 11, 2014 at 02:53:39PM +0800, Thomas Preud'homme wrote: > +2014-03-10 Thomas Preud'homme <thomas.preudhomme@arm.com> > + > + PR tree-optimization/60454 > + * gcc.c-torture/execute/pr60454.c (fake_swap32): Testcase to track > + regression of PR60454. The ChangeLog entry is wrong, the file is new, so you should just say: * gcc.c-torture/execute/pr60454.c: New test. But more importantly: > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/execute/pr60454.c > @@ -0,0 +1,25 @@ > +#include <stdint.h> I'm not sure if all targets even provide stdint.h header, and if all targets have uint32_t type. For most targets gcc now provides stdint.h (or uses glibc stdint.h). So perhaps instead of including stdint.h just do: #ifndef __UINT32_TYPE__ int main () { return 0; } #else typedef __UINT32_TYPE__ uint32_t; ... #endif ? > + > +#define __fake_const_swab32(x) ((uint32_t)( \ > + (((uint32_t)(x) & (uint32_t)0x000000ffUL) << 24) | \ > + (((uint32_t)(x) & (uint32_t)0x0000ff00UL) << 8) | \ > + (((uint32_t)(x) & (uint32_t)0x000000ffUL) << 8) | \ > + (((uint32_t)(x) & (uint32_t)0x0000ff00UL) ) | \ > + (((uint32_t)(x) & (uint32_t)0xff000000UL) >> 24))) > + > +/* Previous version of bswap optimization would detect byte swap when none > + happen. This test aims at catching such wrong detection to avoid > + regressions. */ > + > +uint32_t > +fake_swap32 (uint32_t in) __attribute__ ((noinline, noclone)) > +{ This must not have been tested, this is a syntax error. It should be __attribute__ (noinline, noclone)) uint32_t fake_swap32 (uint32_t in) { ... (the attribute can be after the arguments only for prototypes). > + return __fake_const_swab32 (in); > +} > + > +int main(void) > +{ > + if (fake_swap32 (0x12345678) != 0x78567E12) > + abort (); Use __builtin_abort (); here instead? You aren't including stdlib.h for abort. After fixing up the testcase, you don't need to bootstrap/regtest it again, make check-gcc RUNTESTFLAGS=execute.exp=pr60454.c should be enough. > --- a/gcc/tree-ssa-math-opts.c > +++ b/gcc/tree-ssa-math-opts.c > @@ -1801,7 +1801,9 @@ find_bswap_1 (gimple stmt, struct symbolic_number *n, int limit) > > if (rhs_class == GIMPLE_BINARY_RHS) > { > + int i; > struct symbolic_number n1, n2; > + unsigned HOST_WIDEST_INT mask; > tree source_expr2; > > if (code != BIT_IOR_EXPR) > @@ -1827,6 +1829,15 @@ find_bswap_1 (gimple stmt, struct symbolic_number *n, int limit) > return NULL_TREE; > > n->size = n1.size; > + for (i = 0, mask = 0xff; i < n->size; i++, mask <<= BITS_PER_UNIT) > + { > + unsigned HOST_WIDEST_INT masked1, masked2; > + > + masked1 = n1.n & mask; > + masked2 = n2.n & mask; > + if (masked1 && masked2 && masked1 != masked2) > + return NULL_TREE; > + } > n->n = n1.n | n2.n; > > if (!verify_symbolic_number_p (n, stmt)) The rest looks good to me. Jakub
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 748805e..b6d7d93 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2014-03-07 Thomas Preud'homme <thomas.preudhomme@arm.com> + + PR tree-optimization/60454 + * tree-ssa-math-opts.c (find_bswap_1): Fix bswap detection. + 2014-02-23 David Holsgrove <david.holsgrove@xilinx.com> * config/microblaze/microblaze.md: Correct ashrsi_reg / lshrsi_reg names diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index f3c0c85..b95b2b3 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2014-03-10 Thomas Preud'homme <thomas.preudhomme@arm.com> + + PR tree-optimization/60454 + * gcc.c-torture/execute/pr60454.c (fake_swap32): Testcase to track + regression of PR60454. + 2014-02-23 David Holsgrove <david.holsgrove@xilinx.com> * gcc/testsuite/gcc.target/microblaze/others/mem_reload.c: New test. diff --git a/gcc/testsuite/gcc.c-torture/execute/pr60454.c b/gcc/testsuite/gcc.c-torture/execute/pr60454.c new file mode 100644 index 0000000..e5fbd8f --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr60454.c @@ -0,0 +1,25 @@ +#include <stdint.h> + +#define __fake_const_swab32(x) ((uint32_t)( \ + (((uint32_t)(x) & (uint32_t)0x000000ffUL) << 24) | \ + (((uint32_t)(x) & (uint32_t)0x0000ff00UL) << 8) | \ + (((uint32_t)(x) & (uint32_t)0x000000ffUL) << 8) | \ + (((uint32_t)(x) & (uint32_t)0x0000ff00UL) ) | \ + (((uint32_t)(x) & (uint32_t)0xff000000UL) >> 24))) + +/* Previous version of bswap optimization would detect byte swap when none + happen. This test aims at catching such wrong detection to avoid + regressions. */ + +uint32_t +fake_swap32 (uint32_t in) __attribute__ ((noinline, noclone)) +{ + return __fake_const_swab32 (in); +} + +int main(void) +{ + if (fake_swap32 (0x12345678) != 0x78567E12) + abort (); + return 0; +} diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c index 8e372ed..9ff857c 100644 --- a/gcc/tree-ssa-math-opts.c +++ b/gcc/tree-ssa-math-opts.c @@ -1801,7 +1801,9 @@ find_bswap_1 (gimple stmt, struct symbolic_number *n, int limit) if (rhs_class == GIMPLE_BINARY_RHS) { + int i; struct symbolic_number n1, n2; + unsigned HOST_WIDEST_INT mask; tree source_expr2; if (code != BIT_IOR_EXPR) @@ -1827,6 +1829,15 @@ find_bswap_1 (gimple stmt, struct symbolic_number *n, int limit) return NULL_TREE; n->size = n1.size; + for (i = 0, mask = 0xff; i < n->size; i++, mask <<= BITS_PER_UNIT) + { + unsigned HOST_WIDEST_INT masked1, masked2; + + masked1 = n1.n & mask; + masked2 = n2.n & mask; + if (masked1 && masked2 && masked1 != masked2) + return NULL_TREE; + } n->n = n1.n | n2.n; if (!verify_symbolic_number_p (n, stmt))