| Message ID | 000301cf39e8$84eb6910$8ec23b30$@arm.com |
|---|---|
| State | New |
| Headers | show |
On 7 March 2014 09:35, Thomas Preud'homme <thomas.preudhomme@arm.com> wrote: > +++ b/gcc/testsuite/gcc.dg/optimize-bswapsi-2.c > @@ -0,0 +1,26 @@ > +/* { dg-do compile { target arm*-*-* alpha*-*-* i?86-*-* powerpc*-*-* rs6000-*-* x86_64-*-* s390*-*-* } } */ I'm fairly sure the target list here should include aarch64*-*-*, I tried to apply the patch to my tree to check but failed because the patch appears to have been corrupted by email... Cheers /Marcus
On Fri, Mar 7, 2014 at 12:45 PM, Marcus Shawcroft <marcus.shawcroft@gmail.com> wrote: > On 7 March 2014 09:35, Thomas Preud'homme <thomas.preudhomme@arm.com> wrote: > >> +++ b/gcc/testsuite/gcc.dg/optimize-bswapsi-2.c >> @@ -0,0 +1,26 @@ >> +/* { dg-do compile { target arm*-*-* alpha*-*-* i?86-*-* powerpc*-*-* rs6000-*-* x86_64-*-* s390*-*-* } } */ > > I'm fairly sure the target list here should include aarch64*-*-*, I > tried to apply the patch to my tree to check but failed because the > patch appears to have been corrupted by email... It would be better to make it a runtime testcase that abort()s when miscompiled. Richard. > Cheers > /Marcus
Hi! On Fri, Mar 07, 2014 at 05:35:01PM +0800, Thomas Preud'homme wrote: > --- 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 <<= 8) This should be mask <<= BITS_PER_UNIT for consistency. And, as has been said earlier, the testcase should be a runtime testcase (in gcc.c-torture/execute/), probably with __attribute__((noinline, noclone)) on the function, where main calls the function with a couple of different values, verifies the result and aborts if it is incorrect. 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..d3e0385 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2014-03-07 Thomas Preud'homme <thomas.preudhomme@arm.com> + + PR tree-optimization/60454 + * gcc.dg/optimize-bswapsi-2.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.dg/optimize-bswapsi-2.c b/gcc/testsuite/gcc.dg/optimize-bswapsi-2.c new file mode 100644 index 0000000..f20269e --- /dev/null +++ b/gcc/testsuite/gcc.dg/optimize-bswapsi-2.c @@ -0,0 +1,26 @@ +/* { dg-do compile { target arm*-*-* alpha*-*-* i?86-*-* powerpc*-*-* rs6000-*-* x86_64-*-* s390*-*-* } } */ +/* { dg-require-effective-target stdint_types } */ +/* { dg-options "-O2 -fdump-tree-bswap" } */ +/* { dg-options "-O2 -fdump-tree-bswap -march=z900" { target s390-*-* } } */ + +#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) +{ + return __fake_const_swab32 (in); +} + +/* { dg-final { scan-tree-dump-times "32 bit bswap implementation found at" 0 "bswap" } } */ +/* { dg-final { cleanup-tree-dump "bswap" } } */ diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c index 8e372ed..10a11ec 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 <<= 8) + { + 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))