On Wed, Feb 12, 2025 at 1:14 PM Uros Bizjak <ubiz...@gmail.com> wrote:
>
> The combine pass is trying to combine:
>
> Trying 16, 22, 21 -> 23:
>    16: r104:QI=flags:CCNO>0
>    22: {r120:QI=r104:QI^0x1;clobber flags:CC;}
>       REG_UNUSED flags:CC
>    21: r119:QI=flags:CCNO<=0
>       REG_DEAD flags:CCNO
>    23: {r110:QI=r119:QI|r120:QI;clobber flags:CC;}
>       REG_DEAD r120:QI
>       REG_DEAD r119:QI
>       REG_UNUSED flags:CC
>
> and creates the following two insn sequence:
>
> modifying insn i2    22: r104:QI=flags:CCNO>0
>       REG_DEAD flags:CC
> deferring rescan insn with uid = 22.
> modifying insn i3    23: r110:QI=flags:CCNO<=0
>       REG_DEAD flags:CC
> deferring rescan insn with uid = 23.
>
> where the REG_DEAD note in i2 is not correct, because the flags
> register is still referenced in i3.  In try_combine() megafunction, we
> have this part:
>
> --cut here--
>     /* Distribute all the LOG_LINKS and REG_NOTES from I1, I2, and I3.  */
>     if (i3notes)
>       distribute_notes (i3notes, i3, i3, newi2pat ? i2 : NULL,
>             elim_i2, elim_i1, elim_i0);
>     if (i2notes)
>       distribute_notes (i2notes, i2, i3, newi2pat ? i2 : NULL,
>             elim_i2, elim_i1, elim_i0);
>     if (i1notes)
>       distribute_notes (i1notes, i1, i3, newi2pat ? i2 : NULL,
>             elim_i2, local_elim_i1, local_elim_i0);
>     if (i0notes)
>       distribute_notes (i0notes, i0, i3, newi2pat ? i2 : NULL,
>             elim_i2, elim_i1, local_elim_i0);
>     if (midnotes)
>       distribute_notes (midnotes, NULL, i3, newi2pat ? i2 : NULL,
>             elim_i2, elim_i1, elim_i0);
> --cut here--
>
> where the compiler distributes REG_UNUSED note from i2:
>
>    22: {r120:QI=r104:QI^0x1;clobber flags:CC;}
>       REG_UNUSED flags:CC
>
> via distribute_notes() using the following:
>
> --cut here--
>           /* Otherwise, if this register is now referenced in i2
>          then the register used to be modified in one of the
>          original insns.  If it was i3 (say, in an unused
>          parallel), it's now completely gone, so the note can
>          be discarded.  But if it was modified in i2, i1 or i0
>          and we still reference it in i2, then we're
>          referencing the previous value, and since the
>          register was modified and REG_UNUSED, we know that
>          the previous value is now dead.  So, if we only
>          reference the register in i2, we change the note to
>          REG_DEAD, to reflect the previous value.  However, if
>          we're also setting or clobbering the register as
>          scratch, we know (because the register was not
>          referenced in i3) that it's unused, just as it was
>          unused before, and we place the note in i2.  */
>           if (from_insn != i3 && i2 && INSN_P (i2)
>           && reg_referenced_p (XEXP (note, 0), PATTERN (i2)))
>         {
>           if (!reg_set_p (XEXP (note, 0), PATTERN (i2)))
>             PUT_REG_NOTE_KIND (note, REG_DEAD);
>           if (! (REG_P (XEXP (note, 0))
>              ? find_regno_note (i2, REG_NOTE_KIND (note),
>                         REGNO (XEXP (note, 0)))
>              : find_reg_note (i2, REG_NOTE_KIND (note),
>                       XEXP (note, 0))))
>             place = i2;
>         }
> --cut here--
>
> However, the flags register is not UNUSED (or DEAD), because it is
> used in i3.  The proposed solution is to remove the REG_UNUSED note
> from i2 when the register is also mentioned in i3.

Oops, forgot to include ChangeLog entry:

--cut here--
    PR rtl-optimization/118739

gcc/ChangeLog:

    * combine.cc (distribute_notes) <case REG_UNUSED>: Remove
    REG_UNUSED note from i2 when the register is also mentioned in i3.

gcc/testsuite/ChangeLog:

    * gcc.target/i386/pr118739.c: New test.
--cut here--

>
> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
>
> OK for master and eventual backports?
>
> Uros.

Reply via email to