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