| Summary: | [3.4 Regression] Bitfield operations cause shifts with 0-count to slip through backends | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Giovanni Bajo <giovannibajo> |
| Component: | middle-end | Assignee: | Marek Michalkiewicz <marekm> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | aleph, bernie, bjoern.m.haase, eric.weddington, gcc-bugs, pinskia, sayle |
| Priority: | P3 | Keywords: | missed-optimization |
| Version: | 3.4.2 | ||
| Target Milestone: | 3.4.4 | ||
| Host: | Target: | avr-elf | |
| Build: | Known to work: | 4.0.0 | |
| Known to fail: | 3.4.2 3.4.3 | Last reconfirmed: | 2005-01-08 18:52:50 |
| Attachments: |
Testcase to reproduce the bug
Generated assembly code (with bug!) Patch for 3.4 (also applies to mainline) Superseded by patch for PR19293 |
||
Created attachment 7898 [details]
Testcase to reproduce the bug
Created attachment 7899 [details]
Generated assembly code (with bug!)
Compiler details: Reading specs from /usr/local/avr/lib/gcc/avr/3.4.2/specs Configured with: ../gcc-3.4.2/configure --host=i686-pc-linux-gnu -- prefix=/usr/local/avr --target=avr --enable-languages=c : (reconfigured) ../gcc- 3.4.2/configure --host=i686-pc-linux-gnu --prefix=/usr/local/avr --target=avr -- enable-languages=c,c++ Thread model: single gcc version 3.4.2 Also fails with this pre-release version: avr-gcc (GCC) 3.4.3 20041019 (prerelease) Copyright (C) 2004 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Bootstrapping today's mainline right now. clr r25 ; 46 ashlqi3/4 [length = 1]
(insn 46 65 49 (set (reg:QI 25 r25 [66])
(ashift:QI (reg:QI 25 r25 [66])
(const_int 0 [0x0]))) 57 {ashlqi3} (nil)
(nil))
So there are two problems here, one a target problem in that it should not be generating a clr for this
insn. Two this insn should not be here but that just a misoptimization.
Bernardo's working on a patch. Created attachment 7905 [details] Patch for 3.4 (also applies to mainline) Superseded by patch for PR19293 The shift with zero comes from regmove. Patch posted here: <http://gcc.gnu.org/ml/gcc-patches/2005-01/msg00454.html>. I have the impression that Bug #19329 is the same as bug #19239 (as one might think already when looking at the similarity of the numbers :-) ) 19239, howeverr so far has addressed the issue of *negative* and other really illegal shift counts. In a patch treating both problems correctly, one should possibly implement both: Treatment of negative and other illegal shift counts and positive shift counts. Yours, Björn. Sorry for this: In my posting above, I have misspelled the bug number. I wanted to refer you to bug #19293 (and not #19239, luckyly the number of possible permutations is countable). By the way at #19293, you will also find a patch suggestion that should be eaysily adapted to all of the present shifting problems. Yours again, Björn (In reply to comment #11) > By the way at #19293, you will also find a patch suggestion that should be > eaysily adapted to all of the present shifting problems. I agree PR19293 is a superset of this bug, so I'll close it and withdraw my (already approved) patch. Could you update your patch with a test for <= 0? Please post it in gcc-patches, with me and Marek Michalkiewicz <marekm@amelek.gda.pl> in Cc. I will take care to install your patch in CVS as soon as it gets approved. *** This bug has been marked as a duplicate of 19293 *** Oops, I forgot this bug should stay open until someone figures out why GCC 3.4 leaks through insns with a 0 shift count. I've reclassified the bug as affecting the middle-end. I'm no longer in charge for this bug. (In reply to comment #8) > The shift with zero comes from regmove. Well I did figure out where the shift with zero came from see above but why it comes about I don't know. Subject: Bug 19329 CVSROOT: /cvs/gcc Module name: gcc Changes by: marekm@gcc.gnu.org 2005-01-26 21:44:26 Modified files: gcc : ChangeLog gcc/config/avr : avr.c avr.md Log message: PR target/19293 PR target/19329 * config/avr/avr.c (notice_update_cc): Only set condition code for ashrqi3 if shift count > 0. (out_shift_with_cnt): Handle shift count <= 0 as a no-op. (ashlqi3_out, ashlhi3_out, ashlsi3_out, ashrqi3_out, ashrhi3_out, ashrsi3_out, lshrqi3_out, lshrhi3_out, lshrsi3_out): Handle shift count <= 0 as a no-op, and shift count >= width by copying zero or sign bit to all bits of the result. * config/avr/avr.md (all shifts): Add alternatives for zero shift count, with attribute "length" set to 0 and "cc" set to "none". Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.7290&r2=2.7291 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/avr/avr.c.diff?cvsroot=gcc&r1=1.127&r2=1.128 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/avr/avr.md.diff?cvsroot=gcc&r1=1.47&r2=1.48 Subject: Bug 19329 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_4-branch Changes by: marekm@gcc.gnu.org 2005-01-27 22:36:00 Modified files: gcc : ChangeLog gcc/config/avr : avr.c avr.md Log message: PR target/19293 PR target/19329 * config/avr/avr.c (notice_update_cc): Only set condition code for ashrqi3 if shift count > 0. (out_shift_with_cnt): Handle shift count <= 0 as a no-op. (ashlqi3_out, ashlhi3_out, ashlsi3_out, ashrqi3_out, ashrhi3_out, ashrsi3_out, lshrqi3_out, lshrhi3_out, lshrsi3_out): Handle shift count <= 0 as a no-op, and shift count >= width by copying zero or sign bit to all bits of the result. * config/avr/avr.md (all shifts): Add alternatives for zero shift count, with attribute "length" set to 0 and "cc" set to "none". Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.785&r2=2.2326.2.786 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/avr/avr.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.108.4.3&r2=1.108.4.4 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/avr/avr.md.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.42.4.1&r2=1.42.4.2 Marek, is this bug fixed then? Can we close it? (In reply to comment #18) > Marek, is this bug fixed then? Can we close it? Fixed for the AVR, but the middle-end may still generate these "shift by 0" insns - not sure if all other targets handle them correctly. Fixed then. Roger, do you believe that middle-end is right in generating these shifts by zero? If you believe it is a bug, we can open a new bugreport to track it. |
The testcase I'm going to attach generates a miscompilation on the 3.4 branch. The bug cannot be reproduced on 4.0 but might actually be latent there. The code it's copying 4 1-bit fields from one structure to a different one. The internal bit offsets of the fields are different by just one bit between the two structures. This is an except of the generate code: lsl r18 # source bitfield disposition is 1bit # shifited compared to the original, so GCC # adjust it andi r24,lo8(-25) # clear bits it's going to write mov r25,r18 clr r25 # *BUG* it will always write zero # instead of copying the value andi r25,lo8(16) andi r18,lo8(8) or r24,r18 # write correct bit or r24,r25 # write zero (sigh!)