[Bug c/48197] possible wrong code bug at -O0
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48197 Jakub Jelinek jakub at gcc dot gnu.org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution||FIXED --- Comment #13 from Jakub Jelinek jakub at gcc dot gnu.org 2011-05-05 10:27:16 UTC --- Fixed for 4.6+.
[Bug c/48197] possible wrong code bug at -O0
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48197 --- Comment #12 from Jakub Jelinek jakub at gcc dot gnu.org 2011-03-26 09:23:03 UTC --- Author: jakub Date: Sat Mar 26 09:23:01 2011 New Revision: 171548 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=171548 Log: Backport from mainline 2011-03-20 Jakub Jelinek ja...@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
[Bug c/48197] possible wrong code bug at -O0
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48197 --- Comment #11 from Jakub Jelinek jakub at gcc dot gnu.org 2011-03-21 17:57:41 UTC --- Author: jakub Date: Mon Mar 21 17:57:34 2011 New Revision: 171252 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=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
[Bug c/48197] possible wrong code bug at -O0
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48197 Jakub Jelinek jakub at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2011.03.19 11:41:51 AssignedTo|unassigned at gcc dot |jakub at gcc dot gnu.org |gnu.org | Ever Confirmed|0 |1
[Bug c/48197] possible wrong code bug at -O0
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48197 Jakub Jelinek jakub at gcc dot gnu.org changed: What|Removed |Added Status|NEW |ASSIGNED Last reconfirmed|2011-03-19 11:41:51 |
[Bug c/48197] possible wrong code bug at -O0
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48197 --- Comment #8 from Richard Guenther rguenth at gcc dot gnu.org 2011-03-19 12:34:55 UTC --- (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?
[Bug c/48197] possible wrong code bug at -O0
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48197 --- Comment #9 from Jakub Jelinek jakub at gcc dot gnu.org 2011-03-19 15:06:14 UTC --- Created attachment 23723 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=23723 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.
[Bug c/48197] possible wrong code bug at -O0
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48197 --- Comment #10 from Jakub Jelinek jakub at gcc dot gnu.org 2011-03-19 15:17:06 UTC --- 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.
[Bug c/48197] possible wrong code bug at -O0
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48197 --- Comment #1 from Andrew Pinski pinskia at gcc dot gnu.org 2011-03-18 19:56:42 UTC --- 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.
[Bug c/48197] possible wrong code bug at -O0
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48197 --- Comment #2 from John Regehr regehr at cs dot utah.edu 2011-03-18 20:04:04 UTC --- (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).
[Bug c/48197] possible wrong code bug at -O0
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48197
--- Comment #3 from Jakub Jelinek jakub at gcc dot gnu.org 2011-03-18
20:09:18 UTC ---
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.
[Bug c/48197] possible wrong code bug at -O0
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48197 --- Comment #4 from John Regehr regehr at cs dot utah.edu 2011-03-18 20:12:31 UTC --- Thanks Jakub, I was just about to send the same example!
[Bug c/48197] possible wrong code bug at -O0
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48197
--- Comment #5 from John Regehr regehr at cs dot utah.edu 2011-03-18 20:14:51
UTC ---
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.
[Bug c/48197] possible wrong code bug at -O0
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48197 --- Comment #6 from John Regehr regehr at cs dot utah.edu 2011-03-18 20:23:32 UTC --- Bleh... nevermind the longer test, it carries along my misunderstanding of the sign extension. Anyway, thanks!
[Bug c/48197] possible wrong code bug at -O0
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48197 Jakub Jelinek jakub at gcc dot gnu.org changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #7 from Jakub Jelinek jakub at gcc dot gnu.org 2011-03-18 20:54:07 UTC --- 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.
