regehr@john-home:~$ current-gcc -v Using built-in specs. COLLECT_GCC=current-gcc COLLECT_LTO_WRAPPER=/home/regehr/z/compiler-install/gcc-r161813-install/libexec/gcc/i686-pc-linux-gnu/4.6.0/lto-wrapper Target: i686-pc-linux-gnu Configured with: ../configure --with-libelf=/usr/local --enable-lto --prefix=/home/regehr/z/compiler-install/gcc-r161813-install --program-prefix=r161813- --enable-languages=c,c++ Thread model: posix gcc version 4.6.0 20100704 (experimental) (GCC) regehr@john-home:~$ current-gcc -O1 small.c -o small regehr@john-home:~$ ./small checksum g_40 = 274686410 regehr@john-home:~$ current-gcc -O2 small.c -o small regehr@john-home:~$ ./small checksum g_40 = -1 regehr@john-home:~$ cat small.c extern int printf (__const char *__restrict __format, ...); static char foo (char si1, char si2) { return si1* si2; } const volatile unsigned int g_2[8][3] = {{0L, 0L, 0L}, {0L, 0L, 0L}, {0L, 0L, 0L}, {0L, 0L, 0L}, {0L, 0L, 0L}, {0L, 0L, 0L}, {0L, 0L, 0L}, {0L, 0L, 0L}}; long long g_29 = 1; int g_40 = 0x105F61CAL; int *g_39 = &g_40; volatile int * volatile g_88[1] = {0}; volatile int g_429[5] = {1L, 1L, 1L, 1L, 1L}; int main(void) { int * const l_353 = &g_40; int l_414 = 0xF5B296C2L; if (!(g_2[5][2])) { int l_420 = 0x0332F5C8L; if (((foo (l_420, (*l_353))) > (!-10L))) { for (l_414 = 0; l_414 < 1; l_414 += 1) { g_88[l_414] = &g_429[2]; } (*g_39) = -1; } } printf("checksum g_40 = %d\n", g_40); return g_29; }
Shorter testcase: extern void abort (void); static char foo (char si1, char si2) { return si1 * si2; } int a = 0x105F61CA; int main (void) { int b = 0x0332F5C8; if (foo (b, a) > 0) abort (); return 0; } Works with -O2 -fwrapv.
Not sure whether the testcase is valid or not. The multiplication using char variables on both sides (and likewise for result) is: -54 * -56 (= 3024), but (char) 3024 is -48. For int that would be clear undefined behavior, but for char the multiplication is promoted to int, so it is (char) (int * int). *.ccp1 has: char D.2741; char D.2741; char si2; int D.2727; char D.2726; int a.0; <bb 2>: a.0_2 = a; D.2726_3 = (char) a.0_2; D.2727_4 = (int) D.2726_3; si2_11 = (char) D.2727_4; D.2741_12 = si2_11 * -56; D.2741_13 = D.2741_12; D.2741_7 = D.2741_13; if (D.2741_7 > 0) but forwprop1 uses if (si2_11 < 0) test instead. If the testcase is valid, we shouldn't assume undefined overflow for char and short operations.
The bug is that we have in .original { return si1 * si2; } while it should have been { return (char)((unsigned char) si1 * (unsigned char) si2); } which is premature optimization by convert_to_integer, short-cutting the correctly implemented fold of (signed char)((int) si1 * (int) si2) to the above. Index: gcc/convert.c =================================================================== --- gcc/convert.c (revision 161840) +++ gcc/convert.c (working copy) @@ -774,7 +774,8 @@ convert_to_integer (tree type, tree expr || ((!TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg0)) || !TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg1))) && (ex_form == PLUS_EXPR - || ex_form == MINUS_EXPR))) + || ex_form == MINUS_EXPR + || ex_form == MULT_EXPR))) typex = unsigned_type_for (typex); else typex = signed_type_for (typex); fixes this.
Subject: Bug 44828 Author: rguenth Date: Tue Jul 6 13:37:58 2010 New Revision: 161869 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=161869 Log: 2010-07-06 Richard Guenther <rguenther@suse.de> PR middle-end/44828 * convert.c (convert_to_integer): Watch out for overflowing MULT_EXPR as well. * gcc.c-torture/execute/pr44828.c: New testcase. Added: trunk/gcc/testsuite/gcc.c-torture/execute/pr44828.c Modified: trunk/gcc/ChangeLog trunk/gcc/convert.c trunk/gcc/testsuite/ChangeLog
Fixed on trunk. The problem is latent everywhere but the optimization doesn't happen for 4.2.4 or earlier. Which makes it a regression.
(In reply to comment #2) > Not sure whether the testcase is valid or not. The multiplication using char > variables on both sides (and likewise for result) is: -54 * -56 (= 3024), > but (char) 3024 is -48. For int that would be clear undefined behavior, but > for char the multiplication is promoted to int, so it is (char) (int * int). My students and I had to argue about this and read the standard before submitting this bug report. But I'm almost certain the testcase is valid.
Jakub's test case still fails on powerpc*-linux because we default to unsigned char. I think the obvious fix is to just pass -fsigned-char in dg-options.
Subject: Bug 44828 Author: bergner Date: Thu Jul 8 04:12:04 2010 New Revision: 161942 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=161942 Log: PR middle-end/44828 * gcc.c-torture/execute/pr44828.x: New file. Added: trunk/gcc/testsuite/gcc.c-torture/execute/pr44828.x Modified: trunk/gcc/testsuite/ChangeLog
The fix in Comment #8 fixes the test case on systems that have a default of unsigned char (eg, powerpc*-linux).
I'd say the test instead just should use signed char instead of char.
Subject: Bug 44828 Author: rguenth Date: Thu Jul 8 11:56:08 2010 New Revision: 161951 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=161951 Log: 2010-07-08 Richard Guenther <rguenther@suse.de> Backport from mainline 2010-05-27 Richard Guenther <rguenther@suse.de> PR tree-optimization/44284 * tree-vect-stmts.c (vectorizable_assignment): Handle sign-changing conversions as simple copy. * gcc.dg/vect/vect-118.c: New testcase. * gcc.dg/vect/bb-slp-20.c: Adjust. * gcc.dg/vect/no-section-anchors-vect-36.c: Likewise. * gcc.dg/vect/slp-9.c: Likewise. * gcc.dg/vect/slp-reduc-4.c: Likewise. * gcc.dg/vect/vect-10.c: Likewise. * gcc.dg/vect/vect-109.c: Likewise. * gcc.dg/vect/vect-12.c: Likewise. * gcc.dg/vect/vect-36.c: Likewise. * gcc.dg/vect/vect-7.c: Likewise. * gcc.dg/vect/vect-iv-8.c: Likewise. * gcc.dg/vect/vect-multitypes-10.c: Likewise. * gcc.dg/vect/vect-multitypes-13.c: Likewise. * gcc.dg/vect/vect-multitypes-14.c: Likewise. * gcc.dg/vect/vect-multitypes-15.c: Likewise. * gcc.dg/vect/vect-multitypes-7.c: Likewise. * gcc.dg/vect/vect-multitypes-8.c: Likewise. * gcc.dg/vect/vect-multitypes-9.c: Likewise. * gcc.dg/vect/vect-reduc-dot-s16b.c: Likewise. * gcc.dg/vect/vect-reduc-dot-s8a.c: Likewise. * gcc.dg/vect/vect-reduc-dot-s8b.c: Likewise. * gcc.dg/vect/vect-reduc-dot-u16b.c: Likewise. * gcc.dg/vect/vect-strided-a-u32-mult.c: Likewise. * gcc.dg/vect/vect-strided-u32-mult.c: Likewise. * gcc.dg/vect/vect-widen-mult-s16.c: Likewise. * gcc.dg/vect/vect-widen-mult-s8.c: Likewise. * gcc.dg/vect/vect-widen-mult-sum.c: Likewise. * gcc.dg/vect/vect-widen-mult-u16.c: Likewise. 2010-07-06 Richard Guenther <rguenther@suse.de> PR middle-end/44828 * convert.c (convert_to_integer): Watch out for overflowing MULT_EXPR as well. * gcc.c-torture/execute/pr44828.c: New testcase. Added: branches/gcc-4_5-branch/gcc/testsuite/gcc.c-torture/execute/pr44828.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-118.c Modified: branches/gcc-4_5-branch/gcc/ChangeLog branches/gcc-4_5-branch/gcc/convert.c branches/gcc-4_5-branch/gcc/testsuite/ChangeLog branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/bb-slp-20.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/no-section-anchors-vect-36.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/slp-9.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-10.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-109.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-12.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-36.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-7.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-iv-8.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-multitypes-10.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-multitypes-13.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-multitypes-14.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-multitypes-15.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-multitypes-7.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-multitypes-8.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-multitypes-9.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s16a.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s16b.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8b.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8c.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-u16b.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-strided-a-u32-mult.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-strided-u32-mult.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-widen-mult-s16.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-widen-mult-s8.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-widen-mult-sum.c branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/vect-widen-mult-u16.c branches/gcc-4_5-branch/gcc/tree-vect-stmts.c
Fixed for 4.5.1 as well. Backporting to older branches will regress vectorization if the fix for PR44284 isn't backported as well which IMHO isn't appropriate.
Subject: Bug 44828 Author: bergner Date: Thu Jul 8 14:17:52 2010 New Revision: 161956 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=161956 Log: PR middle-end/44828 * gcc.c-torture/execute/pr44828.c (foo): Use signed char. * gcc.c-torture/execute/pr44828.x: Revert. Removed: trunk/gcc/testsuite/gcc.c-torture/execute/pr44828.x Modified: trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.c-torture/execute/pr44828.c
Fixed for 4.5.1+.