Use-and-clobber insns and REG_DEAD notes

[Ulrich Weigand]

Michael Matz wrote:

> Just to be sure, you have a (read) reference to CC, plus an extra clobber
> for CC in the insn, yes?  Like
> 
> parallel[
>   (set (bla) (plus (reg blubb) (reg CC))
>   (clobber (reg CC))]
> 
> And no explicit set of CC, right?

Exactly.

> > However, when some other insn is combined into it, combine.c thinks it
> > does not need to distribute the REG_DEAD note into the insn (because
> > dead_or_set_p returns true),
> 
> Hmm, I'm not sure I understand.  Insn X is combined _into_ such a
> cc-use-clobber insn Y?  To where should the REG_DEAD note (from insn Y) be
> distributed.  insn X will be deleted if the combining is successful.

Sorry for being unclear.  The situation is like this:  I'm compiling
this code (which is a standard way of building up a 64-bit addition
from 32-bit additions):

  unsigned int x, y;
  x = al + bl;
  y = ah + bh + (x < al);

Before combine, this results in the following insn list (after ifcvt
has recognized and converted the conditional):

(insn 18 12 20 0 (parallel [
            (set (reg/v:SI 49 [ x ])
                (plus:SI (reg/v:SI 44 [ al ])
                    (reg/v:SI 48 [ bl ])))
            (clobber (reg:CC 33 %cc))
        ]) 138 {addsi3} (insn_list 11 (insn_list 8 (nil)))
    (expr_list:REG_DEAD (reg/v:SI 48 [ bl ])
        (expr_list:REG_UNUSED (reg:CC 33 %cc)
            (nil))))

(insn 20 18 38 0 (parallel [
            (set (reg:SI 51)
                (plus:SI (reg/v:SI 42 [ ah ])
                    (reg/v:SI 46 [ bh ])))
            (clobber (reg:CC 33 %cc))
        ]) 138 {addsi3} (insn_list 6 (insn_list 10 (nil)))
    (expr_list:REG_DEAD (reg/v:SI 42 [ ah ])
        (expr_list:REG_DEAD (reg/v:SI 46 [ bh ])
            (expr_list:REG_UNUSED (reg:CC 33 %cc)
                (nil)))))

(insn 38 20 39 0 (set (reg:CCL1 33 %cc)
        (compare:CCL1 (reg/v:SI 49 [ x ])
            (reg/v:SI 44 [ al ]))) 33 {*cmpsi_ccur} (insn_list 18 (nil))
    (expr_list:REG_DEAD (reg/v:SI 44 [ al ])
        (nil)))

(insn 39 38 27 0 (parallel [
            (set (reg/v:SI 50 [ y ])
                (plus:SI (plus:SI (reg:SI 51)
                        (const_int 0 [0x0]))
                    (ltu:SI (reg:CCL1 33 %cc)
                        (const_int 0 [0x0]))))
            (clobber (reg:CC 33 %cc))
        ]) 144 {addsi3_alc} (insn_list 20 (insn_list 38 (nil)))
    (expr_list:REG_DEAD (reg:SI 51)
        (expr_list:REG_DEAD (reg:CCL1 33 %cc)
            (expr_list:REG_UNUSED (reg:CC 33 %cc)
                (nil)))))

Note both REG_DEAD and REG_UNUSED on the %cc register.

Combine is now supposed to collapse 18 into 38 and 20 into 39,
resulting in:

(insn 38 20 39 0 (parallel [
            (set (reg:CCL1 33 %cc)
                (compare:CCL1 (plus:SI (reg:SI 5 %r5 [ al+4 ])
                        (mem/f:SI (plus:DI (reg/f:DI 32 %ap)
                                (const_int 4 [0x4])) [4 bl+0 S4 A32]))
                    (reg:SI 5 %r5 [ al+4 ])))
            (set (reg/v:SI 49 [ x ])
                (plus:SI (reg:SI 5 %r5 [ al+4 ])
                    (mem/f:SI (plus:DI (reg/f:DI 32 %ap)
                            (const_int 4 [0x4])) [4 bl+0 S4 A32])))
        ]) 129 {*addsi3_carry1_cc} (nil)
    (expr_list:REG_DEAD (reg:DI 5 %r5 [ al ])
        (nil)))

(insn 39 38 27 0 (parallel [
            (set (reg/v:SI 50 [ y ])
                (plus:SI (plus:SI (reg:SI 4 %r4 [ ah+4 ])
                        (reg:SI 6 %r6 [ bh+4 ]))
                    (ltu:SI (reg:CCL1 33 %cc)
                        (const_int 0 [0x0]))))
            (clobber (reg:CC 33 %cc))
        ]) 144 {addsi3_alc} (nil)
    (expr_list:REG_DEAD (reg:DI 4 %r4 [ ah ])
        (expr_list:REG_DEAD (reg:DI 6 %r6 [ bh ])
            (expr_list:REG_UNUSED (reg:CC 33 %cc)
                (nil)))))

(it has in fact replaced some of the registers as well, but this
shouldn't make any difference).

The interesting case is now the combination of 20 into 39, and in 
particular what is supposed to happen with the notes that these
instructions were carrying

    (expr_list:REG_DEAD (reg/v:SI 42 [ ah ])
        (expr_list:REG_DEAD (reg/v:SI 46 [ bh ])
            (expr_list:REG_UNUSED (reg:CC 33 %cc)

    (expr_list:REG_DEAD (reg:SI 51)
        (expr_list:REG_DEAD (reg:CCL1 33 %cc)
            (expr_list:REG_UNUSED (reg:CC 33 %cc)

What combine does is to take all of these notes and try to figure
out which of them are still needed.  The REG_DEAD of 42 and 46 are
kept (renamed to reg 4 and reg 6).  The two REG_UNUSED of %cc are
merged into one.  The interesting case is what is supposed to happen
with the REG_DEAD of %cc.

Here, the distribute_notes routine of combine.c contains logic that
says, if dead_or_set_p returns true of %cc at the new insn (which it
does due to the clobber), then the REG_DEAD note is superfluous and
can be omitted.

However, this results in a different set of notes than what flow.c
would have arrived at had it processed the result of combine.c.
And in fact, during scheduling, data flow analysis *is* redone,
and does result in the additional dead note, which causes an
assertion in sched to fail (number of dead notes before != number
of dead notes after).

> And even if flow would behave as described above (not emitting the
> REG_UNUSED note), CC would _still_ be dead_or_set_p().  After all it _is_
> set (clobbered, and thereby implicitely even dead).

Yes, the logic in combine.c would still work the same.  The problem
is that this logic (if dead_or_set_p then we don't need REG_DEAD notes)
is *not* the same as the logic applied by flow.c (there we have: if the
register is *set* in the insn, we don't need a REG_DEAD note, but if
it is clobbered, we still want one).

Either the one or the other needs to be fixed, the question is which
one.  So, should insn 39 above carry a REG_DEAD note for %cc or not?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de