Using this version/config: ~~~~~~~~~~~~~~~~~ Using built-in specs. Target: avr Configured with: ../gcc-4.1.2/configure --prefix=/c/WinAVR --target=avr --enable -languages=c,c++ --with-dwarf2 --enable-win32-registry=WinAVR-20070525 --disable -nls --with-gmp=/usr/local --with-mpfr=/usr/local --enable-doc --disable-libssp Thread model: single gcc version 4.1.2 (WinAVR 20070525) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ I compiled the source using this command line: avr-gcc -S -Os test.c -mmcu=atmega16 These examples also suffers from double and instruction missed (bugID 11259) The key point is that writing an expression like: tmp = 0; if(var&(1<<N)) tmp|=1; results in an N shift (done by loop if N larger then 3) While rewriting to: tmp = 0; if(var>>N)&1) tmp|=1; This uses a swap instruction and then shifts. It's optimal (except from the same loss as bugID 12259 for N >= 4) Maybe this gives a hint on where the shifting is generally going wrong. I tried 3 approaches in the test.c file. Another interresting thing is the removal off the extra and instructions in the last examples in the file.
Created attachment 14053 [details] examples of good extraction and bad extraction Adding the test file showing the missed optimization
It might be interesting if you tried 4.2.1.
Wouter, please attach the assembly output that you are getting for your test.c file using 4.1.2. That way we can compare it to other compiler versions. Thanks, Eric
Created attachment 14105 [details] Assembler output of testcase using 4.1.2 This is the requested assembler output that Eric asked for
Created attachment 14106 [details] Correct assembler output of test case for 4.1.2.
Created attachment 14107 [details] Test case assembler output for 4.2.1. Not really any better than 4.1.2.
Created attachment 14108 [details] Test case assembler output for 4.3.0 20070817 snapshot. Again, only marginally better.
Created attachment 21787 [details] Test case assembler output for 4.5.0.
Lot better code size in gcc-4.5.0 and above [head]. See the attachment in comment #8.
Created attachment 24264 [details] Proposed patch. Proposed Patch (also compatible with older versions of GCC). The insn needs at most 3 instructions and does not put pressure on d-regs. * config/avr/avr.md ("*extzv"): New insn.
Created attachment 24265 [details] Assembler output from 4.7.0 (r173832) with patch applied. Assembler output from 4.7.0 (r173832) with patch applied. There's not much room for improvement left now.
Created attachment 24277 [details] Proposed patch Patch that is less fuzzy in its attributes.
Yep it looks a lot better now. The if statements could be optimized into the equivalent shift instructions but that is not a AVR backend problem I guess. I noticed that the disassembly shows wrong lengths for some outputs of extzv. Is that a problem?
(In reply to comment #13) > Yep it looks a lot better now. > The if statements could be optimized into the equivalent shift instructions but > that is not a AVR backend problem I guess. Shifting generally is more expensive instead of bit extracting; at least if the bit offset is known at compile time. > I noticed that the disassembly shows wrong lengths for some outputs of extzv. > Is that a problem? It's not a problem if the sequence actually printed is not greater than the instruction length reported. If the reported instruction length was strinct greater, an assembler error could occur because relative jump offets migh be out of scope. The only case where the instruction length is smaller than reported is if IN_REG=OUT_REG and BITPOS=4 (sequence is swap + andi 1). The instruction length for lsr + andi 1 (IN_REG=OUT_REG, BITPOS=1) is already corrected in the patch.
*** Bug 49446 has been marked as a duplicate of this bug. ***
Created attachment 24563 [details] pr33049.diff Patch also covering PR49446. Tested without regression against SVN 175201 PR target/33049 * config/avr/avr.md (extzv): New expander. (*extzv, *extzv.qihi1, *extzv.qihi2): New insn-and-split.
Author: gjl Date: Tue Jun 21 17:30:54 2011 New Revision: 175269 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=175269 Log: PR target/33049 * config/avr/avr.md (extzv): New expander. (*extzv): New insn. (*extzv.qihi1, *extzv.qihi2): New insn-and-split. * config/avr/constraints.md (C04): New constraint. * doc/md.texi (Machine Constraints): Document it. Modified: trunk/gcc/ChangeLog trunk/gcc/config/avr/avr.md trunk/gcc/config/avr/constraints.md trunk/gcc/doc/md.texi
Closing as resolved+fixed according to applied patch.
See also http://lists.gnu.org/archive/html/avr-gcc-list/2008-12/msg00009.html