[Bug target/36133] GCC creates suboptimal ASM : Code includes unneeded TST instructions
--- Comment #11 from ams at gcc dot gnu dot org 2008-11-19 12:03 --- The patch just committed should fix this issue. The patch discussion is here: http://gcc.gnu.org/ml/gcc-patches/2008-11/msg00641.html -- ams at gcc dot gnu dot org changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36133
[Bug target/36133] GCC creates suboptimal ASM : Code includes unneeded TST instructions
--- Comment #10 from ams at gcc dot gnu dot org 2008-11-19 11:25 --- Subject: Bug 36133 Author: ams Date: Wed Nov 19 11:23:28 2008 New Revision: 141999 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=141999 Log: 2008-11-19 Andrew Stubbs <[EMAIL PROTECTED]> gcc/ PR target/36133 * config/m68k/m68k.h (CC_OVERFLOW_UNUSABLE, CC_NO_CARRY): New defines. * config/m68k/m68k.c (notice_update_cc): Set cc_status properly for shift instructions. * config/m68k/m68k.md: Adjust all conditional branches that use the carry and overflow flags so they understand CC_OVERFLOW_UNUSABLE. gcc/testsuite/ PR target/36133 * gcc.target/m68k/pr36133.c: New test. Added: trunk/gcc/testsuite/gcc.target/m68k/pr36133.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/m68k/m68k.c trunk/gcc/config/m68k/m68k.h trunk/gcc/config/m68k/m68k.md trunk/gcc/testsuite/ChangeLog -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36133
[Bug target/36133] GCC creates suboptimal ASM : Code includes unneeded TST instructions
--- Comment #9 from ams at gcc dot gnu dot org 2008-11-10 13:01 --- > > This tst instruction is unneeded as the LSR is setting the flags correctly > > already. > > This is NOT correct. LSL does write to the condition codes, but not all of it. > In particular, the bit involved in the not-equal test is not set. Hmm, on re-reading the manual, I seem to have misunderstood this. The description of the instruction states certain bits are set, but then, on the next page the table shows all the bits being updated. I'm going to need to think about this one some more. Sorry for the noise :( Andrew -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36133
[Bug target/36133] GCC creates suboptimal ASM : Code includes unneeded TST instructions
--- Comment #8 from gunnar at greyhound-data dot com 2008-11-10 12:54 --- (In reply to comment #7) > (In reply to comment #4) > > There are two causes where GCC generates unneeded TST instructions. > > A) General arithmetic > > lsr.l #1,D0 > > tst.l d0 > > jbne ... > > > > This tst instruction is unneeded as the LSR is setting the flags correctly > > already. > > This is NOT correct. LSL does write to the condition codes, but not all of it. > In particular, the bit involved in the not-equal test is not set. > > This TST *is* required. What you is say is not correct. The bit involved in the not-eval test is the "Z-Bit" Both the LSR and TST do set the Z-Bit, 100% equally. The TST instruction in the example is 100% unneeded and NOT required. Please check the official 68K documentation for the which flags the conditinal branch instruction BCC tests (in this case BNE), and verify the behavior of TST and LSR in regards of setting these bits. Best Regards Gunnar von Boehn -- gunnar at greyhound-data dot com changed: What|Removed |Added Status|RESOLVED|UNCONFIRMED Resolution|INVALID | http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36133
[Bug target/36133] GCC creates suboptimal ASM : Code includes unneeded TST instructions
--- Comment #7 from ams at gcc dot gnu dot org 2008-11-10 12:29 --- (In reply to comment #4) > There are two causes where GCC generates unneeded TST instructions. > A) General arithmetic > lsr.l #1,D0 > tst.l d0 > jbne ... > > This tst instruction is unneeded as the LSR is setting the flags correctly > already. This is NOT correct. LSL does write to the condition codes, but not all of it. In particular, the bit involved in the not-equal test is not set. This TST *is* required. > B) subq.l #1,D1 > tst.l d1 > jbne ... > > This unneeded TST is related to the compile option used. > If you compile the source with "-O2" then the second unneeded TST instructions > are not included in the source. Please check the code mode carefully. This TST is not related to the SUBQ instruction. This TST is related to the LSL instruction - the TST is a branch target. The compiler has combined the initial and subsequent loop condition tests into one test, thus saving space, as per the -Os requirements. In this case there is an alternative, more speed efficient solution, but that is a separate problem, and not a misuse of the TST instruction. As you say, the compiler gets it right at -O2, so the machine description is clearly correct, in this respect. -- ams at gcc dot gnu dot org changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution||INVALID http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36133
[Bug target/36133] GCC creates suboptimal ASM : Code includes unneeded TST instructions
--- Comment #6 from gunnar at greyhound-data dot com 2008-06-12 14:26 --- Andreas, Could you have a look at this? Cheers Gunnar -- gunnar at greyhound-data dot com changed: What|Removed |Added CC||schwab at suse dot de http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36133
[Bug target/36133] GCC creates suboptimal ASM : Code includes unneeded TST instructions
--- Comment #5 from gunnar at greyhound-data dot com 2008-06-05 12:07 --- Please find below a proposed patch. The patch will making GCC aware that shift does set the CC already and the TST is not needed in this case. The same example could be used to used to make GCC aware of the CC set by other instructions. Index: gcc/config/m68k/m68k.md === *** gcc/config/m68k/m68k.md.orig2008-05-30 10:00:55.0 +0200 --- gcc/config/m68k/m68k.md 2008-06-04 17:01:11.0 +0200 *** *** 5198,5203 --- 5198,5215 [(set_attr "type" "shift") (set_attr "opy" "2")]) + (define_insn "*lshrsi3_cc" + [(set (cc0) + (lshiftrt:SI (match_operand:SI 1 "register_operand" "0") +(match_operand:SI 2 "general_operand" "dI"))) +(set (match_operand:SI 0 "register_operand" "=d") + (lshiftrt:SI (match_dup 1) +(match_dup 2)))] + "" + "lsr%.l %2,%0" + [(set_attr "type" "shift") +(set_attr "opy" "2")]) + (define_insn "lshrhi3" [(set (match_operand:HI 0 "register_operand" "=d") (lshiftrt:HI (match_operand:HI 1 "register_operand" "0") -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36133
[Bug target/36133] GCC creates suboptimal ASM : Code includes unneeded TST instructions
--- Comment #4 from gunnar at greyhound-data dot com 2008-06-04 09:29 --- I want to add that this wrong behavior is partly related to the compile option "-Os". There are two causes where GCC generates unneeded TST instructions. A) General arithmetic lsr.l #1,D0 tst.l d0 jbne ... This tst instruction is unneeded as the LSR is setting the flags correctly already. B) subq.l #1,D1 tst.l d1 jbne ... This unneeded TST is related to the compile option used. If you compile the source with "-O2" then the second unneeded TST instructions are not included in the source. It seems to me that a general important optimizations step - which used to be in "Os" in GCC 2.9 was removed from "Os" causing GCC to generate worse code now. Can you please be so kind and correct this? I believe that this issue is quite serious for the performance of the generated code. 1st The unneeded TST instructions are increasing code size, which is important in embedded environments. 2nd There are case were the instruction which really did set the condition codes correctly in the first place is far enough away from the conditional branch and no CC trashing instruction in between them - so that the instruction fetcher can 100% correctly predict the branch and fold it away completely. The unneeded TST instruction makes branch folding impossible and requires the CPU to guess the branch instead. This will cause a serious performance impact in case of mispredicting the branch. It should be clear that the unneeded TST instruction doas not only bloat the code but the above mentioned conditions can serious degrade the performance as well, depending on your used CPU of course. In the light of this, wouldn't it might sense to increase the Severity of this issue? Regards Gunnar -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36133
[Bug target/36133] GCC creates suboptimal ASM : Code includes unneeded TST instructions
--- Comment #3 from gunnar at greyhound-data dot com 2008-05-28 16:14 --- (In reply to comment #1) > It would have been nice to check at least gcc 4.3 (or better current trunk). > I've verified with latest source gcc source "version 4.4.0 20080523 (experimental) (GCC)" The problem that GCC used totally unneeded TST instructions is still in the current source. Regards Gunnar -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36133
[Bug target/36133] GCC creates suboptimal ASM : Code includes unneeded TST instructions
--- Comment #2 from hp at gcc dot gnu dot org 2008-05-23 23:04 --- This could be a duplicate of PR20211. -- hp at gcc dot gnu dot org changed: What|Removed |Added CC||hp at gcc dot gnu dot org http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36133
[Bug target/36133] GCC creates suboptimal ASM : Code includes unneeded TST instructions
--- Comment #1 from rguenth at gcc dot gnu dot org 2008-05-07 19:33 --- It would have been nice to check at least gcc 4.3 (or better current trunk). -- rguenth at gcc dot gnu dot org changed: What|Removed |Added Severity|normal |enhancement http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36133
