The current gcc trunk miscompiles the following code on x86_64-linux-gnu at -O3 in both 32-bit and 64-bit modes. This is a regression from 5.2.x. $ gcc-trunk -v Using built-in specs. COLLECT_GCC=gcc-trunk COLLECT_LTO_WRAPPER=/usr/local/gcc-trunk/libexec/gcc/x86_64-pc-linux-gnu/6.0.0/lto-wrapper Target: x86_64-pc-linux-gnu Configured with: ../gcc-trunk/configure --prefix=/usr/local/gcc-trunk --enable-languages=c,c++ --disable-werror --enable-multilib Thread model: posix gcc version 6.0.0 20151002 (experimental) [trunk revision 228389] (GCC) $ $ gcc-trunk -O2 small.c; ./a.out 0 $ gcc-5.2 -O3 small.c; ./a.out 0 $ $ gcc-trunk -O3 small.c $ ./a.out 1 $ ----------------------------------- int printf (const char *, ...); int a, b; short c; int main () { int j, d = 1; for (; c >= 0; c++) { a = d; d = 0; if (b) { printf ("%d", 0); if (j) printf ("%d", 0); } } printf ("%d\n", d); return 0; }
Confirmed.
Started with r226901.
Created attachment 36448 [details] Patch I'm testing to fix the bug This testcase invokes undefined behavior because of the overflow on the iterator, but the cause of the problem is undefined behavior we introduce ourselves by unswitching the loop on the uninitialized variable j. Things go down the hill from there, as we uncprop j's default def in the j==0 branch into one of D's phi nodes in that version of the loop, then we coalesce j's default def with some versions of d (we only detect conflicts for parms' and results' default defs), including the one passed to printf. init-regs adds zero initialization for j, the loop-entry initialization of d with 1 overrides it, and the assignment to zero within the loop, that uncprop had turned into a copy from j (known to be zero in that branch), is optimized out because of the coalescing. So the initialization of d to 1 prevails. Preventing coalescing by detecting conflicts with non-params would also avoid the symptom, but since accessing uninitialized variables is undefined behavior, we shouldn't have to worry about that. This is only relevant in this case because we introduce the undefined access in the first place, so that's what the patch fixes.
Use ssa_undefined_value_p (..., true), otherwise looks sensible. I suppose we could unswitch on the value if the condition is always executed in the loop but then that's hardly a useful optimization (on undefined values).
> This testcase invokes undefined behavior because of the overflow on the > iterator, ... Just a quick comment that the testcase doesn't have undefined behaviors. As the variable c is of type short, there shouldn't be signed overflow --- the operations will be performed on its signed extended integer value and then truncated back to a short. Thanks for looking into and fixing the issue.
Below is another testcase that I believe exposes the same issue: --------------------------------------- int a = 2, b = 1, c = 1; int fn1 () { int d; for (; a; a--) { for (d = 0; d < 4; d++) { int k; if (c < 1) if (k) c = 0; if (b) continue; return 0; } b = !1; } return 0; } int main () { fn1 (); if (a != 1) __builtin_abort (); return 0; }
Author: aoliva Date: Fri Oct 9 12:18:24 2015 New Revision: 228650 URL: https://gcc.gnu.org/viewcvs?rev=228650&root=gcc&view=rev Log: [PR67828] don't unswitch on default defs of non-parms for gcc/ChangeLog PR rtl-optimizatoin/67828 * tree-ssa-loop-unswitch.c: Include tree-ssa.h. (tree_may_unswitch_on): Don't unswitch on expressions involving undefined values. for gcc/testsuite/ChangeLog PR rtl-optimization/67828 * gcc.dg/torture/pr67828.c: New. Added: trunk/gcc/testsuite/gcc.dg/torture/pr67828.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-ssa-loop-unswitch.c
Fixed