On Tue, Mar 11, 2014 at 02:53:39PM +0800, Thomas Preud'homme wrote:
> +2014-03-10 Thomas Preud'homme <thomas.preudho...@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
