Re: [PATCH] Adjust builtin-bswap-6/7

2014-04-08 Thread Richard Biener 
On Mon, Apr 7, 2014 at 6:22 PM, Andreas Krebbel
kreb...@linux.vnet.ibm.com 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?

Hmm, they passed before your change.  Do you mean that this was
by accident (and only because of the special return value)?  If so then
the patch is ok.

Thanks,
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

Re: [PATCH] Adjust builtin-bswap-6/7

2014-04-08 Thread Jakub Jelinek 
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.

 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

Re: [PATCH] Adjust builtin-bswap-6/7

2014-04-08 Thread Richard Biener 
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

Re: [PATCH] Adjust builtin-bswap-6/7

2014-04-08 Thread Jakub Jelinek 
On Tue, Apr 08, 2014 at 10:26:30AM +0200, Richard Biener wrote:
 On Mon, Apr 7, 2014 at 6:22 PM, Andreas Krebbel
 kreb...@linux.vnet.ibm.com 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?
 
 Hmm, they passed before your change.  Do you mean that this was
 by accident (and only because of the special return value)?  If so then
 the patch is ok.

The reason why it worked with the if ... return 1; else return 0; case is
that in that case it has already been expanded as store flag insn and thus
in that case ce1 pass didn't discover the conditional move there, thus no
dead code waiting to be eliminated after ce1 and still present during
combine pass.

Another alternative for -O2 would be -O -fno-if-conversion I guess.

OT, when touching the testcase, I'd say it would be better if you've
converted it to single dg-options + /* { dg-additional-options -march=z900 { 
target s390*-*-* } } */

  --- 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

Re: [PATCH] Adjust builtin-bswap-6/7

2014-04-08 Thread Andreas Krebbel 
On 04/08/2014 10:41 AM, Jakub Jelinek wrote:
 On Tue, Apr 08, 2014 at 10:26:30AM +0200, Richard Biener wrote:
 On Mon, Apr 7, 2014 at 6:22 PM, Andreas Krebbel
 kreb...@linux.vnet.ibm.com 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?

 Hmm, they passed before your change.  Do you mean that this was
 by accident (and only because of the special return value)?  If so then
 the patch is ok.
 
 The reason why it worked with the if ... return 1; else return 0; case is
 that in that case it has already been expanded as store flag insn and thus
 in that case ce1 pass didn't discover the conditional move there, thus no
 dead code waiting to be eliminated after ce1 and still present during
 combine pass.
 
 Another alternative for -O2 would be -O -fno-if-conversion I guess.

I could also revert the testcase changes and add -mbranch-cost=2 for s390?!

 
 OT, when touching the testcase, I'd say it would be better if you've
 converted it to single dg-options + /* { dg-additional-options -march=z900 
 { target s390*-*-* } } */

Ok.

Bye,

-Andreas-

 
 --- 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

Re: [PATCH] Adjust builtin-bswap-6/7

2014-04-08 Thread Jakub Jelinek 
On Tue, Apr 08, 2014 at 10:53:19AM +0200, Andreas Krebbel wrote:
 On 04/08/2014 10:41 AM, Jakub Jelinek wrote:
  On Tue, Apr 08, 2014 at 10:26:30AM +0200, Richard Biener wrote:
  On Mon, Apr 7, 2014 at 6:22 PM, Andreas Krebbel
  kreb...@linux.vnet.ibm.com 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?
 
  Hmm, they passed before your change.  Do you mean that this was
  by accident (and only because of the special return value)?  If so then
  the patch is ok.
  
  The reason why it worked with the if ... return 1; else return 0; case is
  that in that case it has already been expanded as store flag insn and thus
  in that case ce1 pass didn't discover the conditional move there, thus no
  dead code waiting to be eliminated after ce1 and still present during
  combine pass.
  
  Another alternative for -O2 would be -O -fno-if-conversion I guess.
 
 I could also revert the testcase changes and add -mbranch-cost=2 for s390?!

That doesn't seem to work (at least for me with cross-compiler to s390x).

  
  OT, when touching the testcase, I'd say it would be better if you've
  converted it to single dg-options + /* { dg-additional-options 
  -march=z900 { target s390*-*-* } } */
 
 Ok.

Thanks.

Jakub

Re: [PATCH] Adjust builtin-bswap-6/7

2014-04-08 Thread Andreas Krebbel 
On 04/08/2014 11:12 AM, Jakub Jelinek wrote:
 On Tue, Apr 08, 2014 at 10:53:19AM +0200, Andreas Krebbel wrote:
 On 04/08/2014 10:41 AM, Jakub Jelinek wrote:
 On Tue, Apr 08, 2014 at 10:26:30AM +0200, Richard Biener wrote:
 On Mon, Apr 7, 2014 at 6:22 PM, Andreas Krebbel
 kreb...@linux.vnet.ibm.com 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?

 Hmm, they passed before your change.  Do you mean that this was
 by accident (and only because of the special return value)?  If so then
 the patch is ok.

 The reason why it worked with the if ... return 1; else return 0; case is
 that in that case it has already been expanded as store flag insn and thus
 in that case ce1 pass didn't discover the conditional move there, thus no
 dead code waiting to be eliminated after ce1 and still present during
 combine pass.

 Another alternative for -O2 would be -O -fno-if-conversion I guess.

 I could also revert the testcase changes and add -mbranch-cost=2 for s390?!
 
 That doesn't seem to work (at least for me with cross-compiler to s390x).

Seems to be -mbranch-cost=0 this time.

 

 OT, when touching the testcase, I'd say it would be better if you've
 converted it to single dg-options + /* { dg-additional-options 
 -march=z900 { target s390*-*-* } } */

 Ok.
 
 Thanks.
 
   Jakub

Re: [PATCH] Adjust builtin-bswap-6/7

2014-04-08 Thread Jakub Jelinek 
On Tue, Apr 08, 2014 at 11:21:56AM +0200, Andreas Krebbel wrote:
 On 04/08/2014 11:12 AM, Jakub Jelinek wrote:
  On Tue, Apr 08, 2014 at 10:53:19AM +0200, Andreas Krebbel wrote:
  On 04/08/2014 10:41 AM, Jakub Jelinek wrote:
  On Tue, Apr 08, 2014 at 10:26:30AM +0200, Richard Biener wrote:
  On Mon, Apr 7, 2014 at 6:22 PM, Andreas Krebbel
  kreb...@linux.vnet.ibm.com 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?
 
  Hmm, they passed before your change.  Do you mean that this was
  by accident (and only because of the special return value)?  If so then
  the patch is ok.
 
  The reason why it worked with the if ... return 1; else return 0; case is
  that in that case it has already been expanded as store flag insn and thus
  in that case ce1 pass didn't discover the conditional move there, thus no
  dead code waiting to be eliminated after ce1 and still present during
  combine pass.
 
  Another alternative for -O2 would be -O -fno-if-conversion I guess.
 
  I could also revert the testcase changes and add -mbranch-cost=2 for s390?!
  
  That doesn't seem to work (at least for me with cross-compiler to s390x).
 
 Seems to be -mbranch-cost=0 this time.

Indeed, that works too.

Thus, please commit any of these variants, if you go for
/* { dg-additional-options -mbranch-cost=0 { target s390*-*-* } } */
plus reverting your earlier changes, you can also consider
addition of two new tests that would contain the return {1,2} and have -O2.

Jakub

Re: [PATCH] Adjust builtin-bswap-6/7

2014-04-08 Thread Andreas Krebbel 
Hi,

what about that one?

Tested on x86_64, s390, and s390x.

Bye,

-Andreas-

2014-04-08  Andreas Krebbel  andreas.kreb...@de.ibm.com

* gcc.dg/builtin-bswap-6.c: Use -mbranch-cost=0 for s390.
* gcc.dg/builtin-bswap-7.c: Likewise.

Revert
2014-04-04  Andreas Krebbel  andreas.kreb...@de.ibm.com

* gcc.dg/builtin-bswap-6.c: Adjust return value to disable GCC
optimization.
* gcc.dg/builtin-bswap-7.c: Likewise.

diff --git a/gcc/testsuite/gcc.dg/builtin-bswap-6.c 
b/gcc/testsuite/gcc.dg/builtin-bswap-6.c
index 6f0c782..efda870 100644
--- a/gcc/testsuite/gcc.dg/builtin-bswap-6.c
+++ b/gcc/testsuite/gcc.dg/builtin-bswap-6.c
@@ -1,11 +1,10 @@
 /* { 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-*-* } } */
 
-/* The test intentionally returns 1/2 instead of the obvious 0/1 to
-   prevent GCC from calculating the return value with arithmetic
-   instead of a comparison.  */
+/* The branch cost setting prevents the return value from being
+   calculated with arithmetic instead of doing a compare.  */
+/* { dg-additional-options -march=z900 -mbranch-cost=0 { target s390*-*-* } 
} */
 
 #include stdint.h
 
@@ -15,28 +14,28 @@ int foo1 (uint32_t a)
 {
   if (BS (a) == 0xA)
 return 1;
-  return 2;
+  return 0;
 }
 
 int foo2 (uint32_t a)
 {
   if (BS (a) != 0xA)
 return 1;
-  return 2;
+  return 0;
 }
 
 int foo3 (uint32_t a, uint32_t b)
 {
   if (BS (a) == BS (b))
 return 1;
-  return 2;
+  return 0;
 }
 
 int foo4 (uint32_t a, uint32_t b)
 {
   if (BS (a) != BS (b))
 return 1;
-  return 2;
+  return 0;
 }
 
 /* { dg-final { scan-rtl-dump-not bswapsi combine } } */
diff --git a/gcc/testsuite/gcc.dg/builtin-bswap-7.c 
b/gcc/testsuite/gcc.dg/builtin-bswap-7.c
index 0eecdd8..035c736 100644
--- a/gcc/testsuite/gcc.dg/builtin-bswap-7.c
+++ b/gcc/testsuite/gcc.dg/builtin-bswap-7.c
@@ -3,9 +3,9 @@
 /* { dg-require-effective-target lp64 } */
 /* { dg-options -O -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
-   instead of a comparison.  */
+/* The branch cost setting prevents the return value from being
+   calculated with arithmetic instead of doing a compare.  */
+/* { dg-additional-options -mbranch-cost=0 { target s390x-*-* } } */
 
 #include stdint.h
 
@@ -15,28 +15,28 @@ int foo1 (uint64_t a)
 {
   if (BS (a) == 0xA)
 return 1;
-  return 2;
+  return 0;
 }
 
 int foo2 (uint64_t a)
 {
   if (BS (a) != 0xA)
 return 1;
-  return 2;
+  return 0;
 }
 
 int foo3 (uint64_t a, uint64_t b)
 {
   if (BS (a) == BS (b))
 return 1;
-  return 2;
+  return 0;
 }
 
 int foo4 (uint64_t a, uint64_t b)
 {
   if (BS (a) != BS (b))
 return 1;
-  return 2;
+  return 0;
 }
 
 /* { dg-final { scan-rtl-dump-not bswapdi combine } } */

Re: [PATCH] Adjust builtin-bswap-6/7

2014-04-08 Thread Jakub Jelinek 
On Tue, Apr 08, 2014 at 12:41:41PM +0200, Andreas Krebbel wrote:
 what about that one?
 
 Tested on x86_64, s390, and s390x.

LGTM, thanks.

 2014-04-08  Andreas Krebbel  andreas.kreb...@de.ibm.com
 
   * gcc.dg/builtin-bswap-6.c: Use -mbranch-cost=0 for s390.
   * gcc.dg/builtin-bswap-7.c: Likewise.
 
   Revert
   2014-04-04  Andreas Krebbel  andreas.kreb...@de.ibm.com
 
   * gcc.dg/builtin-bswap-6.c: Adjust return value to disable GCC
   optimization.
   * gcc.dg/builtin-bswap-7.c: Likewise.

Jakub

Re: [PATCH] Adjust builtin-bswap-6/7

2014-04-08 Thread Andreas Krebbel 
On Tue, Apr 08, 2014 at 12:41:02PM +0200, Jakub Jelinek wrote:
 Thus, please commit any of these variants, if you go for
 /* { dg-additional-options -mbranch-cost=0 { target s390*-*-* } } */
 plus reverting your earlier changes, you can also consider
 addition of two new tests that would contain the return {1,2} and have -O2.

I've just committed the following patch:

2014-04-08  Andreas Krebbel  andreas.kreb...@de.ibm.com

PR rtl-optimization/60776
* gcc.dg/builtin-bswap-6.c: Use -mbranch-cost=0 for s390.
* gcc.dg/builtin-bswap-7.c: Likewise.
* gcc.dg/builtin-bswap-6a.c: New testcase.
* gcc.dg/builtin-bswap-7a.c: New testcase.

Revert
2014-04-04  Andreas Krebbel  andreas.kreb...@de.ibm.com

* gcc.dg/builtin-bswap-6.c: Adjust return value to disable GCC
optimization.
* gcc.dg/builtin-bswap-7.c: Likewise.

diff --git a/gcc/testsuite/gcc.dg/builtin-bswap-6.c 
b/gcc/testsuite/gcc.dg/builtin-bswap-6.c
index 6f0c782..efda870 100644
--- a/gcc/testsuite/gcc.dg/builtin-bswap-6.c
+++ b/gcc/testsuite/gcc.dg/builtin-bswap-6.c
@@ -1,11 +1,10 @@
 /* { 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-*-* } } */
 
-/* The test intentionally returns 1/2 instead of the obvious 0/1 to
-   prevent GCC from calculating the return value with arithmetic
-   instead of a comparison.  */
+/* The branch cost setting prevents the return value from being
+   calculated with arithmetic instead of doing a compare.  */
+/* { dg-additional-options -march=z900 -mbranch-cost=0 { target s390*-*-* } 
} */
 
 #include stdint.h
 
@@ -15,28 +14,28 @@ int foo1 (uint32_t a)
 {
   if (BS (a) == 0xA)
 return 1;
-  return 2;
+  return 0;
 }
 
 int foo2 (uint32_t a)
 {
   if (BS (a) != 0xA)
 return 1;
-  return 2;
+  return 0;
 }
 
 int foo3 (uint32_t a, uint32_t b)
 {
   if (BS (a) == BS (b))
 return 1;
-  return 2;
+  return 0;
 }
 
 int foo4 (uint32_t a, uint32_t b)
 {
   if (BS (a) != BS (b))
 return 1;
-  return 2;
+  return 0;
 }
 
 /* { dg-final { scan-rtl-dump-not bswapsi combine } } */
diff --git a/gcc/testsuite/gcc.dg/builtin-bswap-6a.c 
b/gcc/testsuite/gcc.dg/builtin-bswap-6a.c
new file mode 100644
index 000..f93bcde
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/builtin-bswap-6a.c
@@ -0,0 +1,44 @@
+/* { dg-do compile { target arm*-*-* alpha*-*-* i?86-*-* powerpc*-*-* 
rs6000-*-* x86_64-*-* s390*-*-* } } */
+/* { dg-require-effective-target stdint_types } */
+/* { dg-options -O2 -fdump-rtl-combine } */
+/* { dg-additional-options -march=z900 { target s390-*-* } } */
+
+/* The test is similiar to builtin-bswap-6.c but returns 1/2 instead
+   of 0/1 to prevent GCC from calculating the return value with
+   arithmetic instead of a comparison.  This requires the optimization
+   level to be bumped up to -O2 at least for x86_64.  */
+
+#include stdint.h
+
+#define BS(X) __builtin_bswap32(X)
+
+int foo1 (uint32_t a)
+{
+  if (BS (a) == 0xA)
+return 1;
+  return 2;
+}
+
+int foo2 (uint32_t a)
+{
+  if (BS (a) != 0xA)
+return 1;
+  return 2;
+}
+
+int foo3 (uint32_t a, uint32_t b)
+{
+  if (BS (a) == BS (b))
+return 1;
+  return 2;
+}
+
+int foo4 (uint32_t a, uint32_t b)
+{
+  if (BS (a) != BS (b))
+return 1;
+  return 2;
+}
+
+/* { dg-final { scan-rtl-dump-not bswapsi combine } } */
+/* { dg-final { cleanup-rtl-dump combine } } */
diff --git a/gcc/testsuite/gcc.dg/builtin-bswap-7.c 
b/gcc/testsuite/gcc.dg/builtin-bswap-7.c
index 0eecdd8..035c736 100644
--- a/gcc/testsuite/gcc.dg/builtin-bswap-7.c
+++ b/gcc/testsuite/gcc.dg/builtin-bswap-7.c
@@ -3,9 +3,9 @@
 /* { dg-require-effective-target lp64 } */
 /* { dg-options -O -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
-   instead of a comparison.  */
+/* The branch cost setting prevents the return value from being
+   calculated with arithmetic instead of doing a compare.  */
+/* { dg-additional-options -mbranch-cost=0 { target s390x-*-* } } */
 
 #include stdint.h
 
@@ -15,28 +15,28 @@ int foo1 (uint64_t a)
 {
   if (BS (a) == 0xA)
 return 1;
-  return 2;
+  return 0;
 }
 
 int foo2 (uint64_t a)
 {
   if (BS (a) != 0xA)
 return 1;
-  return 2;
+  return 0;
 }
 
 int foo3 (uint64_t a, uint64_t b)
 {
   if (BS (a) == BS (b))
 return 1;
-  return 2;
+  return 0;
 }
 
 int foo4 (uint64_t a, uint64_t b)
 {
   if (BS (a) != BS (b))
 return 1;
-  return 2;
+  return 0;
 }
 
 /* { dg-final { scan-rtl-dump-not bswapdi combine } } */
diff --git a/gcc/testsuite/gcc.dg/builtin-bswap-7a.c 
b/gcc/testsuite/gcc.dg/builtin-bswap-7a.c
new file mode 100644
index 000..d77bd47
--- /dev/null
+++

Re: [PATCH] Adjust builtin-bswap-6/7

2014-04-07 Thread Richard Biener 
On Fri, Apr 4, 2014 at 7:51 PM, Jeff Law l...@redhat.com wrote:
 On 04/04/14 10:18, Andreas Krebbel wrote:

 Hi,

 the attached patch modifies the builtin-bswap-6/7 testcases in order
 to prevent GCC from using math instead of a compare.  Only with a
 compare the folding in combine actually takes place.

 Whether the return value is produce with a compare or not depends
 again on the value of branch cost.

 Ideally we would be able to do the folding also with the math trick
 but it is probably not that easy since we have already lost the
 information that in the end all we need is a 0 or a 1.

 Ok?

 Bye,

 -Andreas-

 2014-04-04  Andreas Krebbel  andreas.kreb...@de.ibm.com

 * gcc.dg/builtin-bswap-6.c: Adjust return value to disable GCC
 optimization.
 * gcc.dg/builtin-bswap-7.c: Likewise.

 OK.

The adjusted testcases now fail on x86_64/i?86 at least.  See PR60776.

Richard.

 Jeff

Re: [PATCH] Adjust builtin-bswap-6/7

2014-04-07 Thread Andreas Krebbel 
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?

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

Re: [PATCH] Adjust builtin-bswap-6/7

2014-04-04 Thread Jeff Law 

On 04/04/14 10:18, Andreas Krebbel wrote:

Hi,

the attached patch modifies the builtin-bswap-6/7 testcases in order
to prevent GCC from using math instead of a compare.  Only with a
compare the folding in combine actually takes place.

Whether the return value is produce with a compare or not depends
again on the value of branch cost.

Ideally we would be able to do the folding also with the math trick
but it is probably not that easy since we have already lost the
information that in the end all we need is a 0 or a 1.

Ok?

Bye,

-Andreas-

2014-04-04  Andreas Krebbel  andreas.kreb...@de.ibm.com

* gcc.dg/builtin-bswap-6.c: Adjust return value to disable GCC
optimization.
* gcc.dg/builtin-bswap-7.c: Likewise.

OK.
Jeff