With gcc-4.5.2, even with gcc-4.5-20110310, the following line of code gets miscompiled (See attachment) : size -= nblk * 512; size and nblk are 64 bits. gcc tries to compute -nblk. nblk is stored in r0,r1. gcc produces the following asm code : rsbs r1, r0, #0 rsc r2, r1, #0 So r1 gets corrupted by the rsbs insn, thus getting wrong value in r2. I'm building the asm code with : gcc -O2 -march=armv5t list2.i -o - -S When I use : gcc -O2 -fno-cse-follow-jumps -march=armv5t list2.i -o - -S I get : rsbs r0, r0, #0 rsc r1, r1, #0 which is fine.
Created attachment 23636 [details] test file
I get correct-looking code on armv5tel-linux with vanilla gcc-4.6-20110305 and gcc-4.5-20110310, with 4.4 the code looks different but not obviously broken. Can you make the test case standalone and include a runtime check for the possible miscompilation? How was your gcc configured?
(In reply to comment #2) > I get correct-looking code on armv5tel-linux with vanilla gcc-4.6-20110305 and > gcc-4.5-20110310, with 4.4 the code looks different but not obviously broken. I said -march=armv5t (or -march=armv4t I guess). It's important as -march=armv5te won't trigger the bug. > > Can you make the test case standalone and include a runtime check for the > possible miscompilation? My test case was to build tar and run the delete01.at check but it's not really a standalone test. So far, I've not managed to have a test case small than compiling the preprocessed file. > > How was your gcc configured? I reproduce it with my gcc and debian gcc 4.5.2 so I believe this rules out misconfiguration.
I have a runtime test case (given test file + separate file with main and missing support functions) that works with gcc-4.5-20110310 -march=armv5te but fails with -march=armv5t. gcc-4.4.5 and 4.6-20110305 work with both armv5te and armv5t.
Created attachment 23637 [details] standalone test case Reduced standalone test case. Fails with gcc-4.5-20110310 -march=armv5t, works with -march=armv5te. Also works with gcc-4.4 and gcc-4.6.
The test case was fixed for 4.6 by r159644. However, that patch <http://gcc.gnu.org/ml/gcc-patches/2010-05/msg01528.html> was described as a minor improvement to double-word register allocation and not a correctness fix, so the underlying bug may be latent on trunk. I'll investigate some more tomorrow.
The miscompilation on 4.5 branch started with r154181, the PR42031 fix; see <http://gcc.gnu.org/ml/gcc-patches/2009-11/msg00725.html>. On the standalone test case, r154181 changed the -march=armv5t -O2 code as follows: --- pr48090.s-r154180 2011-03-13 15:39:16.000000000 +0100 +++ pr48090.s-r154181 2011-03-13 15:43:21.000000000 +0100 @@ -81,11 +81,9 @@ mov r6, r1 bl seek_archive cmp r1, #0 - mov r3, r0 - mov r4, r1 blt .L14 - rsbs r1, r3, #0 - rsc r2, r4, #0 + rsbs r1, r0, #0 + rsc r2, r1, #0 mov r4, r2, asl #9 mov r3, r1, asl #9 orr r4, r4, r1, lsr #23 which breaks the second 'rsc' insn since it now reads the updated r1 value instead of the original one.
The test case broke due to the combination of r154181 and r147087. r147087 is Kazu's "expmed.c: Improve multiplication by more constants" patch, see <http://gcc.gnu.org/ml/gcc-patches/2009-04/msg00915.html>. Backporting either of these to 4.4.5 doesn't break the test case, but when both are backported at the same time it breaks. And reverting r147087 from current 4.5 branch unbreaks the test case.
I confirm that backporting r159644 and r159683 make things work. From comment 8, I guess that the bug is still there and that one can still hit it sooner or later, right ? (btw, amazing job)
(In reply to comment #9) > I confirm that backporting r159644 and r159683 make things work. From comment > 8, I guess that the bug is still there and that one can still hit it sooner or > later, right ? (btw, amazing job) It probably papers over it as you guessed. This bug lacks proper analysis.
(In reply to comment #10) > (In reply to comment #9) > > I confirm that backporting r159644 and r159683 make things work. From comment > > 8, I guess that the bug is still there and that one can still hit it sooner or > > later, right ? (btw, amazing job) > > It probably papers over it as you guessed. > > This bug lacks proper analysis. The problem is latent in all versions of the compiler and is to do with the pattern *arm_negdi2 and has nothing to do with the afore mentioned revisions. Will submit this after a round of testing. Index: gcc/config/arm/arm.md =================================================================== --- gcc/config/arm/arm.md (revision 172252) +++ gcc/config/arm/arm.md (working copy) @@ -3554,7 +3554,7 @@ ;; The constraints here are to prevent a *partial* overlap (where %Q0 == %R1). ;; The first alternative allows the common case of a *full* overlap. (define_insn "*arm_negdi2" - [(set (match_operand:DI 0 "s_register_operand" "=&r,r") + [(set (match_operand:DI 0 "s_register_operand" "=r,&r") (neg:DI (match_operand:DI 1 "s_register_operand" "0,r"))) (clobber (reg:CC CC_REGNUM))] "TARGET_ARM"
Author: ramana Date: Tue Apr 12 13:42:48 2011 New Revision: 172318 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=172318 Log: Fix PR target/48090 2011-04-12 Ramana Radhakrishnan <ramana.radhakrishnan@linaro.org> PR target/48090 * config/arm/arm.md (*arm_negdi2): Fix early clobber constraints. Modified: trunk/gcc/ChangeLog trunk/gcc/config/arm/arm.md
Author: ramana Date: Tue Apr 12 13:52:46 2011 New Revision: 172320 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=172320 Log: Fix PR target/48090 Modified: branches/gcc-4_5-branch/gcc/ChangeLog branches/gcc-4_5-branch/gcc/config/arm/arm.md
Still need to backport and test on the 4.6 branch. That is next. Ramana
On Tue, Apr 12, 2011 at 01:53:48PM +0000, ramana at gcc dot gnu.org wrote: > Still need to backport and test on the 4.6 branch. That is next. Small procedural note: it is preferred to go trunk -> 4.6 -> 4.5, rather than trunk -> 4.5 -> 4.6, even if the bug is reported against 4.5.
Author: ramana Date: Wed Apr 13 07:41:08 2011 New Revision: 172363 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=172363 Log: Fix PR target/48090 Modified: branches/gcc-4_6-branch/gcc/ChangeLog branches/gcc-4_6-branch/gcc/config/arm/arm.md
(In reply to comment #15) > On Tue, Apr 12, 2011 at 01:53:48PM +0000, ramana at gcc dot gnu.org wrote: > > Still need to backport and test on the 4.6 branch. That is next. > > Small procedural note: it is preferred to go trunk -> 4.6 -> 4.5, rather > than trunk -> 4.5 -> 4.6, even if the bug is reported against 4.5. Yes I know but it was just a function of what tree I had ready to test and finish at that point of time. Will do it in the right order from next time. I don't have a 4.4 tree handy and this bug should be latent there as well - so if someone wants to backport it please do so but I don't have the time to do it in the next couple of weeks. cheers Ramana
Fixed.