The current gcc trunk miscompiles the following code on x86_64-linux at -Os and above in both 32-bit and 64-bit modes. This is a regression from 4.9.x. $ gcc-trunk -v Using built-in specs. COLLECT_GCC=gcc-trunk COLLECT_LTO_WRAPPER=/usr/local/gcc-trunk/libexec/gcc/x86_64-unknown-linux-gnu/4.10.0/lto-wrapper Target: x86_64-unknown-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 4.10.0 20140524 (experimental) [trunk revision 210888] (GCC) $ $ gcc-trunk -O1 small.c; a.out $ gcc-4.9.0 -Os small.c; a.out $ $ gcc-trunk -Os small.c $ a.out Aborted (core dumped) $ --------------------------- short a = -1; int b; char c; int main () { c = a; b = a | c; if (b != -1) __builtin_abort (); return 0; }
Confirmed. Looks like fallout from the bswap improvements.
16 bit load in host endianness found at: b.3_7 = (int) load_dst_10; at least the dumping is confusing as well ;) But the issue seems to be that we are missing that a.0_2 and c.1_3 sign-extend. a.0_2 = a; c.1_3 = (char) a.0_2; c = c.1_3; _5 = (int) a.0_2; _6 = (int) c.1_3; b.3_7 = _6 | _5;
Indeed. I also noticed that the original bswap code would happily accept signed ssa value and signed cast which can lead to disaster. I worked out a patch for this issue that check the sign of the lhs of the bitwise or expression and use the (unsigned_)?int.I_type_node accordingly but I now get bootstrap failure. I'll provide a patch asap.
I finally managed to find the root cause for the bootstrap failure with my current fix. I shall be able to improve my fix and should hopefully be ready tomorrow.
I have a working patch that also pass bootstrap. I'll do a bit more testing and post it for review.
Author: thopre01 Date: Wed Jun 11 10:04:33 2014 New Revision: 211444 URL: http://gcc.gnu.org/viewcvs?rev=211444&root=gcc&view=rev Log: 2014-06-11 Thomas Preud'homme <thomas.preudhomme@arm.com> gcc/ PR tree-optimization/61306 * tree-ssa-math-opts.c (struct symbolic_number): Store type of expression instead of its size. (do_shift_rotate): Adapt to change in struct symbolic_number. Return false to prevent optimization when the result is unpredictable due to arithmetic right shift of signed type with highest byte is set. (verify_symbolic_number_p): Adapt to change in struct symbolic_number. (init_symbolic_number): Likewise. (find_bswap_or_nop_1): Likewise. Return NULL to prevent optimization when the result is unpredictable due to sign extension. gcc/testsuite/ * gcc.c-torture/execute/pr61306-1.c: New test. * gcc.c-torture/execute/pr61306-2.c: Likewise. * gcc.c-torture/execute/pr61306-3.c: Likewise. Added: trunk/gcc/testsuite/gcc.c-torture/execute/pr61306-1.c trunk/gcc/testsuite/gcc.c-torture/execute/pr61306-2.c trunk/gcc/testsuite/gcc.c-torture/execute/pr61306-3.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-ssa-math-opts.c
Although the number of code impacted is bigger since the recent improvement to bswap pass to support memory source, this bug is actually quite old. I'm thus updating the known to work/fail versions accordingly
Author: thopre01 Date: Mon Jun 30 01:58:45 2014 New Revision: 212133 URL: https://gcc.gnu.org/viewcvs?rev=212133&root=gcc&view=rev Log: 2014-06-30 Thomas Preud'homme <thomas.preudhomme@arm.com> Backport from mainline 2014-06-20 Jakub Jelinek <jakub@redhat.com> 2014-06-11 Thomas Preud'homme <thomas.preudhomme@arm.com> gcc/ PR tree-optimization/61306 * tree-ssa-math-opts.c (struct symbolic_number): Store type of expression instead of its size. (do_shift_rotate): Adapt to change in struct symbolic_number. Return false to prevent optimization when the result is unpredictable due to arithmetic right shift of signed type with highest byte is set. (verify_symbolic_number_p): Adapt to change in struct symbolic_number. (find_bswap_1): Likewise. Return NULL to prevent optimization when the result is unpredictable due to sign extension. (find_bswap): Adapt to change in struct symbolic_number. gcc/testsuite/ * gcc.c-torture/execute/pr61306-1.c: New test. * gcc.c-torture/execute/pr61306-2.c: Likewise. * gcc.c-torture/execute/pr61306-3.c: Likewise. Added: branches/gcc-4_9-branch/gcc/testsuite/gcc.c-torture/execute/pr61306-1.c branches/gcc-4_9-branch/gcc/testsuite/gcc.c-torture/execute/pr61306-2.c branches/gcc-4_9-branch/gcc/testsuite/gcc.c-torture/execute/pr61306-3.c Modified: branches/gcc-4_9-branch/gcc/ChangeLog branches/gcc-4_9-branch/gcc/testsuite/ChangeLog branches/gcc-4_9-branch/gcc/tree-ssa-math-opts.c
Author: thopre01 Date: Mon Jun 30 02:11:21 2014 New Revision: 212134 URL: https://gcc.gnu.org/viewcvs?rev=212134&root=gcc&view=rev Log: 2014-06-30 Thomas Preud'homme <thomas.preudhomme@arm.com> Backport from mainline 2014-06-20 Jakub Jelinek <jakub@redhat.com> 2014-06-11 Thomas Preud'homme <thomas.preudhomme@arm.com> gcc/ PR tree-optimization/61306 * tree-ssa-math-opts.c (struct symbolic_number): Store type of expression instead of its size. (do_shift_rotate): Adapt to change in struct symbolic_number. Return false to prevent optimization when the result is unpredictable due to arithmetic right shift of signed type with highest byte is set. (verify_symbolic_number_p): Adapt to change in struct symbolic_number. (find_bswap_1): Likewise. Return NULL to prevent optimization when the result is unpredictable due to sign extension. (find_bswap): Adapt to change in struct symbolic_number. gcc/testsuite/ * gcc.c-torture/execute/pr61306-1.c: New test. * gcc.c-torture/execute/pr61306-2.c: Likewise. * gcc.c-torture/execute/pr61306-3.c: Likewise. Added: branches/gcc-4_8-branch/gcc/testsuite/gcc.c-torture/execute/pr61306-1.c branches/gcc-4_8-branch/gcc/testsuite/gcc.c-torture/execute/pr61306-2.c branches/gcc-4_8-branch/gcc/testsuite/gcc.c-torture/execute/pr61306-3.c Modified: branches/gcc-4_8-branch/gcc/ChangeLog branches/gcc-4_8-branch/gcc/testsuite/ChangeLog branches/gcc-4_8-branch/gcc/tree-ssa-math-opts.c
(In reply to thopre01 from comment #6) > Author: thopre01 > Date: Wed Jun 11 10:04:33 2014 > New Revision: 211444 > > URL: http://gcc.gnu.org/viewcvs?rev=211444&root=gcc&view=rev > Log: > 2014-06-11 Thomas Preud'homme <thomas.preudhomme@arm.com> > > gcc/ > PR tree-optimization/61306 > * tree-ssa-math-opts.c (struct symbolic_number): Store type of > expression instead of its size. > (do_shift_rotate): Adapt to change in struct symbolic_number. Return > false to prevent optimization when the result is unpredictable due to > arithmetic right shift of signed type with highest byte is set. > (verify_symbolic_number_p): Adapt to change in struct symbolic_number. > (init_symbolic_number): Likewise. > (find_bswap_or_nop_1): Likewise. Return NULL to prevent optimization > when the result is unpredictable due to sign extension. > > gcc/testsuite/ > * gcc.c-torture/execute/pr61306-1.c: New test. > * gcc.c-torture/execute/pr61306-2.c: Likewise. > * gcc.c-torture/execute/pr61306-3.c: Likewise. > > Added: > trunk/gcc/testsuite/gcc.c-torture/execute/pr61306-1.c > trunk/gcc/testsuite/gcc.c-torture/execute/pr61306-2.c > trunk/gcc/testsuite/gcc.c-torture/execute/pr61306-3.c > Modified: > trunk/gcc/ChangeLog > trunk/gcc/testsuite/ChangeLog > trunk/gcc/tree-ssa-math-opts.c It seems that after this commit, one SH bswap32 test started to fail. Comparing the two test reports: https://gcc.gnu.org/ml/gcc-testresults/2014-06/msg00961.html https://gcc.gnu.org/ml/gcc-testresults/2014-06/msg01086.html shows a new failure for gcc.target/sh/pr53568-1.c. The function int test_func_02 (int a) { /* 1x swap.w 2x swap.b */ return (((a >> 0) & 0xFF) << 24) | (((a >> 8) & 0xFF) << 16) | (((a >> 16) & 0xFF) << 8) | (((a >> 24) & 0xFF) << 0); } now fails to produce a bswap32.
Confirmed. This is because the compiler will detect that the result of (a >> 8) depends on the sign of a and thus prevent the optimization. Before this check incorrect code could be generated. Of course in this case since the high bit or discarded with a bitwise AND it is safe. It is possible to detect this by marking the byte with unknown content with a special marker (say 0xff) instead of cancelling the optimization and modify a few places which checks for marker. This can be done for 4.10 but seems to me to invasive for 4.8 and 4.9 where this patch was backported. If you disagree with this we can discuss with the release manager once the patch made to see if they would agree for a backport. Thanks for your report.
Original bug fixed.