Re: fold_const.c/tree_swap_operands_p change from Oct 11 breaks m68k

[Richard Henderson <rth at redhat dot com>]

On Sat, Oct 25, 2003 at 01:35:54PM -0600, Roger Sayle wrote:
> (insn 49 38 39 0 (set (reg/s:SI %a0 [32])
>         (reg:SI %d0 [33])) 25 {*m68k.md:760} (insn_list 38 (nil))
>     (nil))
> 
> (insn 39 49 40 0 (set (cc0)
>         (reg:SI %d0 [33])) 3 {*m68k.md:196} (nil)
>     (expr_list:REG_DEAD (reg:SI %d0 [33])
>         (nil)))
[...]
> I'm not really a back-end expert.  Here is the relevant define_insn
> from line 196 of m68k.md.  Can anyone see an obvious mistake?

Let me introduce you to NOTICE_UPDATE_CC.

Incidentally, it's not your bug.  Indeed, your patch let us notice
that %d1 was zero, and compare against zero directly instead of 
loading that into a register.

The intent of NOTICE_UPDATE_CC is to watch when an insn preceeding
a compare generates the same comparison codes as the compare would.
If so, final omits the insn.

  foo:  clr.l %d1
        move.l 4(%sp),%d0
        lsr.l #5,%d0
        move.l %d0,%a0
        jbls .L7
  .L5:  addq.l #1,%d1
        cmp.l %a0,%d1
        jbcs .L5
  .L7:  rts

In this case, we should have noticed that the move there is 
really a movea and does not affect the flags, and that the lsr
does in fact set the condition codes.  The bug would appear to
be that while lsr does set the carry bit, it's the carry out of
the top of the old value, not as would be set for comparison
with zero.

So dropping the compare would be ok here if the branch was signed,
but not unsigned.

Indeed, the bug appears to be that 

      case ASHIFT: case ASHIFTRT: case LSHIFTRT:
      case ROTATE: case ROTATERT:

are not special-cased at all.


r~