On Tue, Apr 8, 2014 at 10:31 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Mon, Apr 07, 2014 at 06:22:14PM +0200, Andreas Krebbel wrote:
>> On Mon, Apr 07, 2014 at 04:19:06PM +0200, Richard Biener wrote:
>> > The adjusted testcases now fail on x86_64/i?86 at least. See PR60776.
>>
>> They seem to require at least -O2 on x86 with that change. Ok to apply?
>
> The reason why the modified test now requires -O2 seems to be that during
> ce1 a conditional move is detected, but there is no DCE to clean stuff
> up before combine at -O1 and thus the bswapsi result pseudo has multiple
> uses.
>
> I guess your patch is ok for now, for 5.0 I'd say we should do it at the
> GIMPLE level, probably not in the optimize_bswap pass (because there it is
> guarded with bswap* insn patterns, while we can certainly optimize this
> for libcalls too), but say fold_builtins pass, when seeing
> __builtin_bswap* builtin that couldn't be folded, just check if the lhs has
> a single use that is a comparison with constant and if so, remove the
> swapping and adjust comparison.
Or just
(match_and_simplify
(eq (bswap @1) (bswap @2))
(eq @1 @2))
and similar patterns which should then catch it everywhere.
Richard.
>> diff --git a/gcc/testsuite/gcc.dg/builtin-bswap-6.c
>> b/gcc/testsuite/gcc.dg/builtin-bswap-6.c
>> index 6f0c782..f346c2f 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-bswap-6.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-bswap-6.c
>> @@ -1,7 +1,7 @@
>> /* { dg-do compile { target arm*-*-* alpha*-*-* i?86-*-* powerpc*-*-*
>> rs6000-*-* x86_64-*-* s390*-*-* } } */
>> /* { dg-require-effective-target stdint_types } */
>> -/* { dg-options "-O -fdump-rtl-combine" } */
>> -/* { dg-options "-O -fdump-rtl-combine -march=z900" { target s390-*-* } } */
>> +/* { dg-options "-O2 -fdump-rtl-combine" } */
>> +/* { dg-options "-O2 -fdump-rtl-combine -march=z900" { target s390-*-* } }
>> */
>>
>> /* The test intentionally returns 1/2 instead of the obvious 0/1 to
>> prevent GCC from calculating the return value with arithmetic
>> diff --git a/gcc/testsuite/gcc.dg/builtin-bswap-7.c
>> b/gcc/testsuite/gcc.dg/builtin-bswap-7.c
>> index 0eecdd8..98529f2 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-bswap-7.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-bswap-7.c
>> @@ -1,7 +1,7 @@
>> /* { dg-do compile { target arm*-*-* alpha*-*-* ia64*-*-* x86_64-*-*
>> s390x-*-* powerpc*-*-* rs6000-*-* } } */
>> /* { dg-require-effective-target stdint_types } */
>> /* { dg-require-effective-target lp64 } */
>> -/* { dg-options "-O -fdump-rtl-combine" } */
>> +/* { dg-options "-O2 -fdump-rtl-combine" } */
>>
>> /* The test intentionally returns 1/2 instead of the obvious 0/1 to
>> prevent GCC from calculating the return value with arithmetic
>
> Jakub
