Re: [PATCH] combine: Discard REG_UNUSED note in i2 when register is also referenced in i3 [PR118739]

[Richard Sandiford]

Uros Bizjak <ubiz...@gmail.com> writes:
> On Wed, Feb 12, 2025 at 4:16 PM Richard Sandiford
> <richard.sandif...@arm.com> wrote:
>>
>> Uros Bizjak <ubiz...@gmail.com> writes:
>> > 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
>>
>> It looks like something has already gone wrong in this sequence,
>> in that insn 21 is using the flags after the register has been clobbered.
>> If the flags result of insn 22 is useful, the insn should be setting the
>> flags using a parallel of two sets.
>
> Please note that the insn sequence before combine pass is correct:
>
>    16: r104:QI=flags:CCNO>0
>    21: r119:QI=flags:CCNO<=0
>       REG_DEAD flags:CCNO
>    22: {r120:QI=r104:QI^0x1;clobber flags:CC;}
>       REG_UNUSED flags:CC
>    23: {r110:QI=r119:QI|r120:QI;clobber flags:CC;}
>       REG_DEAD r120:QI
>       REG_DEAD r119:QI
>       REG_UNUSED flags:CC

Ah, ok!  Sorry for the noise.  I hadn't realised that combine sorted
the instructions into program order only after printing them:

  if (dump_file && (dump_flags & TDF_DETAILS))
    {
      if (i0)
        fprintf (dump_file, "\nTrying %d, %d, %d -> %d:\n",
                 INSN_UID (i0), INSN_UID (i1), INSN_UID (i2), INSN_UID (i3));
      else if (i1)
        fprintf (dump_file, "\nTrying %d, %d -> %d:\n",
                 INSN_UID (i1), INSN_UID (i2), INSN_UID (i3));
      else
        fprintf (dump_file, "\nTrying %d -> %d:\n",
                 INSN_UID (i2), INSN_UID (i3));

      if (i0)
        dump_insn_slim (dump_file, i0);
      if (i1)
        dump_insn_slim (dump_file, i1);
      dump_insn_slim (dump_file, i2);
      dump_insn_slim (dump_file, i3);
    }

  /* If multiple insns feed into one of I2 or I3, they can be in any
     order.  To simplify the code below, reorder them in sequence.  */
  if (i0 && DF_INSN_LUID (i0) > DF_INSN_LUID (i2))
    std::swap (i0, i2);
  if (i0 && DF_INSN_LUID (i0) > DF_INSN_LUID (i1))
    std::swap (i0, i1);
  if (i1 && DF_INSN_LUID (i1) > DF_INSN_LUID (i2))
    std::swap (i1, i2);

Feels like it would be more useful to sort them first.

So yeah, please ignore my comment above.

Thanks,
Richard