Re: Ping: PR25514: A problem with note distribution in combine

[Richard Sandiford <richard at codesourcery dot com>]

Alan Modra <amodra@bigpond.net.au> writes:
> On Wed, May 10, 2006 at 01:47:31PM +0100, Richard Sandiford wrote:
>> > Alan added the comment:
>> >
>> >               /* You might think you could search back from FROM_INSN
>> >                  rather than from I3, but combine tries to split invalid
>> >                  combined instructions.  This can result in the old I2
>> >                  or I1 moving later in the insn sequence.  */
>
> If I remember correctly, I was seeing this sequence
>
> 	i2
> 	other insns
> 	i3
>
> being combined to
>
> 	deleted
> 	other insns
> 	i2+i3
>
> but i2+i3 was invalid so it was split to
>
> 	deleted
> 	other insns
> 	i2
> 	i3

I thought if combine split an insn into two, it placed the pattern
of the first insn in the original i2's PATTERN.  I didn't think it
inserted new insns like this.  (Assuming we're talking about the
split_insns code in try_combine here.)

But your message makes me worry that I didn't get my point across.
For avoidance of doubt: you seem to be giving a specific case where
searching from from_insn rather than i3 is wrong when a REG_EQUAL in
from_insn has not been used.  But my thrust was more that searching
from from_insn rather than i3 wouldn't make much sense _anyway_ when
a REG_EQUAL note in from_insn has not been used:

    Alan added the comment:

              /* You might think you could search back from FROM_INSN
                 rather than from I3, but combine tries to split invalid
                 combined instructions.  This can result in the old I2
                 or I1 moving later in the insn sequence.  */
              for (tem = PREV_INSN (tem); place == 0; tem = PREV_INSN (tem))

    But I don't quite follow that.  I think the idea of the original code
    was this:

      (1) The REG_DEAD note refers to something that was used in the
          right hand side of the old FROM_INSN pattern (if FROM_INSN
          is nonnull)

      (2) The right hand side of that pattern has been combined into
          what is now I3 and I2 (if the latter is nonnull)

      (3) The register used to die at FROM_INSN.

      (4) Therefore, the register now dies at the last use before or
          during I3.  If there is no such use, the definition of the
          register is now unused.

      (5) (4) implies searching back from I3 (== D') looking for the first
          use or set of the REG_DEAD register.  If we find a use, we should
          add the REG_DEAD note there.  If we find a set, we should try to
          delete the set as dead, or add a REG_UNUSED note if that isn't
          possible.

    The search described in (5) could be quite expensive, so
    distribute_notes has some straight-line code to handle common cases.
    One of the straight-line cases is where I3 itself uses the register.
    Having disposed of that, the main loop can start at the insn before
    I3 rather than I3 itself.

    Forgetting about the bug in hand, that approach seems correct to me,
    so I don't really see what the comment is trying to apologise for.
    Conceptually, the search really should start at I3.

...rather than from_insn, when a REG_EQUAL in from_insn has not been used.

    The problem of course is that (2) does not hold in this case.
    The original rhs of FROM_INSN (the one to which the REG_DEAD note
    applies) was _not_ used in the combination; a REG_EQUAL note was
    used instead.  Thus any use of the register after FROM_INSN
    cannot be part of the same live range.
    [...]

Richard