Buggy peephole prevents bootstrap (i386)
Zack Weinberg
zackw@Stanford.EDU
Fri Oct 13 20:35:00 GMT 2000
On Sat, Oct 14, 2000 at 02:13:03PM +1100, Alan Modra wrote:
> On Fri, 13 Oct 2000, Zack Weinberg wrote:
>
> > This transformation was done to save space - the add instruction is
> > three bytes shorter than the corresponding compare. However, add
> > doesn't set the condition codes the same way cmp and sub do. It
> > appears (from staring at the confusing Intel reference manual) that
> > add's condition codes are usable only with the signed conditional
>
> Actually, it's the other way around. You can replace
> cmp $val,%reg
> j<cc> ; cc is one of a,b,ae,be or their aliases
>
> with
> add $-val,%reg
> jn<cc>
>
> only for _unsigned_ comparisons (and val != 0). Signed comparisons get
> the overflow bit wrong. If you don't believe me, run a few cases through
> a debugger - I had to do so to make sure I was correct. ;-)
Ah, so the miscompilation of gen_rtx_CONST_INT is because we aren't
changing j<cc> to jn<cc> then. I got confused by the enormous table
of <cc>s in the architecture manual with no indication of whether
they're signed or unsigned.
The offending peepholes look like
(define_peephole2
[(set (reg 17)
(compare (match_operand:SI 0 "register_operand" "")
(const_int 128)))]
"ix86_match_ccmode (insn, CCmode)
&& find_regno_note (insn, REG_DEAD, true_regnum (operands[0]))"
[(parallel [(set (reg:CC 17)
(compare:CC (neg:SI (const_int -128))
(match_dup 0)))
(clobber (match_dup 0))])]
"")
-- we need to make them permit only unsigned compare and update the
jump insn. I don't know how to do either.
zw
More information about the Gcc-bugs
mailing list