Here gcc's output is the same at all optimization levels. We think the correct answer for this function is "x = 0" because all of the following statements are equivalent: x = (long)0 > ((unsigned int)0 ^ (signed short)y); x = (long)0 > ((unsigned int)0 ^ (signed short)((int)0x8000)); x = (long)0 > ((unsigned int)0 ^ (signed short)0x8000); x = (long)0 > ((unsigned)0 ^ (unsigned)0x8000); x = (long)0 > ((unsigned)0 ^ (unsigned)32768); x = (long)0 > (unsigned)32768; x = (long)0 > (long)32768; x = 0; [regehr@gamow ~]$ current-gcc -O0 small.c -o small [regehr@gamow ~]$ ./small x = 1 [regehr@gamow ~]$ cat small.c static int x = 0; static int y = 0x8000; int printf(const char *format, ...); int main (void) { x = (long)0 > ((unsigned int)0 ^ (signed short)y); printf("x = %d\n", x); return 0; } [regehr@gamow ~]$ current-gcc -v Using built-in specs. COLLECT_GCC=current-gcc COLLECT_LTO_WRAPPER=/uusoc/exports/scratch/regehr/z/compiler-install/gcc-r171139-install/bin/../libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/lto-wrapper Target: x86_64-unknown-linux-gnu Configured with: ../configure --with-libelf=/usr/local --enable-lto --prefix=/home/regehr/z/compiler-install/gcc-r171139-install --program-prefix=r171139- --enable-languages=c,c++ Thread model: posix gcc version 4.7.0 20110318 (experimental) (GCC)
x = (long)0 > ((unsigned int)0 ^ (signed short)0x8000); x = (long)0 > ((unsigned)0 ^ (unsigned)0x8000); I think you missed something here (unsigned)(signed short) still sign extends.
(In reply to comment #1) > x = (long)0 > ((unsigned int)0 ^ (signed short)0x8000); > x = (long)0 > ((unsigned)0 ^ (unsigned)0x8000); > I think you missed something here (unsigned)(signed short) still sign extends. Thanks Andrew, I'll look more closely. If GCC is right, then Clang and Intel CC are wrong (assuming all three compilers make the same implementation-dependent decisions for integers on x86-64, which I was under the impression they did).
That's true, the step from the 3rd to 4th line is wrong. But that doesn't mean that on LP64 targets it should print 1. On: extern void abort (void); static int y = 0x8000; int main () { if (0LL > (0U ^ (short)0x8000)) abort (); if (0LL > (0U ^ (short)y)) abort (); return 0; } the first test doesn't abort, the second one does.
Thanks Jakub, I was just about to send the same example!
Here's a test case: int printf(const char *format, ...); int main (void) { int y = 0x8000; int x1 = (long)0 > ((unsigned int)0 ^ (signed short)y); int x2 = (long)0 > ((unsigned int)0 ^ (signed short)((int)0x8000)); int x3 = (long)0 > ((unsigned int)0 ^ (signed short)0x8000); int x4 = (long)0 > ((unsigned)0 ^ (unsigned)y); int x5 = (long)0 > ((unsigned)0 ^ (unsigned)32768); int x6 = (long)0 > (unsigned)32768; int x7 = (long)0 > (long)32768; int x8 = 0; printf("%d %d %d %d %d %d %d %d\n", x1, x2, x3, x4, x5, x6, x7, x8); return 0; } And here's the output: [regehr@gamow ~]$ current-gcc -O0 small.c -o small -Wall [regehr@gamow ~]$ ./small 1 0 0 0 0 0 0 0 I think 0 should be assigned into all of x1..x8.
Bleh... nevermind the longer test, it carries along my misunderstanding of the sign extension. Anyway, thanks!
I think the bug is in shorten_compare. We are called for GT_EXPR with op0 0LL and op1 (unsigned) (short) ((short) 0 ^ (short) y) with long long result type. ^ above is BIT_XOR_EXPR with short type (shortened earlier, but that's fine, (unsigned) (short) ((short) 0 ^ (short y)) is still (unsigned) (short) y, i.e. sign extending y from 16 bits to 32 bits. In the above the (short) cast doesn't really exist, it is simply a NOP_EXPR with unsigned int type of BIT_XOR_EXPR with short int type. get_narrower gives us 0LL (no change, unsignedp0 set to 0) and the short int BIT_XOR_EXPR (again, with unsignedp1 set to 0). That is IMHO also fine, unsignedp1 says that the BIT_XOR_EXPR needs to be sign extended to the type. But in the end shorten_compare says that the comparison just should be done in short int, which is wrong.
(In reply to comment #7) > I think the bug is in shorten_compare. > We are called for GT_EXPR with op0 0LL and op1 (unsigned) (short) ((short) 0 ^ > (short) y) > with long long result type. ^ above is BIT_XOR_EXPR with short type (shortened > earlier, but that's fine, (unsigned) (short) ((short) 0 ^ (short y)) is still > (unsigned) (short) y, i.e. sign extending y from 16 bits to 32 bits. In the > above the (short) cast doesn't really exist, it is simply a NOP_EXPR with > unsigned int type of BIT_XOR_EXPR with short int type. > get_narrower gives us 0LL (no change, unsignedp0 set to 0) and the short int > BIT_XOR_EXPR (again, with unsignedp1 set to 0). That is IMHO also fine, > unsignedp1 says that the BIT_XOR_EXPR needs to be sign extended to the type. > > But in the end shorten_compare says that the comparison just should be done in > short int, which is wrong. Ah, one of my special friends and endless source of wrong-code bugs ;) Can we just remove this premature frontend optimization?
Created attachment 23723 [details] gcc47-pr48197.patch Maybe, but it might not be very easy, as it is related to diagnostics too (both this routine issues warnings and I believe some warnings like -Wconversion rely on the shortening too). In the mean time, here is an untested fix.
BTW, this failed already in gcc 2.7.2.3, so doesn't seem to be a regression. I think we want to fix it in 4.6 nevertheless, but only after 4.6.0 is released.
Author: jakub Date: Mon Mar 21 17:57:34 2011 New Revision: 171252 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=171252 Log: PR c/42544 PR c/48197 * c-common.c (shorten_compare): If primopN is first sign-extended to opN and then zero-extended to result type, set primopN to opN. * gcc.c-torture/execute/pr42544.c: New test. * gcc.c-torture/execute/pr48197.c: New test. Added: trunk/gcc/testsuite/gcc.c-torture/execute/pr42544.c trunk/gcc/testsuite/gcc.c-torture/execute/pr48197.c Modified: trunk/gcc/ChangeLog trunk/gcc/c-family/c-common.c trunk/gcc/testsuite/ChangeLog
Author: jakub Date: Sat Mar 26 09:23:01 2011 New Revision: 171548 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=171548 Log: Backport from mainline 2011-03-20 Jakub Jelinek <jakub@redhat.com> PR c/42544 PR c/48197 * c-common.c (shorten_compare): If primopN is first sign-extended to opN and then zero-extended to result type, set primopN to opN. * gcc.c-torture/execute/pr42544.c: New test. * gcc.c-torture/execute/pr48197.c: New test. Added: branches/gcc-4_6-branch/gcc/testsuite/gcc.c-torture/execute/pr42544.c branches/gcc-4_6-branch/gcc/testsuite/gcc.c-torture/execute/pr48197.c Modified: branches/gcc-4_6-branch/gcc/ChangeLog branches/gcc-4_6-branch/gcc/c-family/c-common.c branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
Fixed for 4.6+.