On 3/16/25 11:20 AM, Andrew Pinski wrote:

gcc/ChangeLog:

       PR rtl-optimization/118914
       * combine.cc (recog_for_combine): Add old_nregs and new_nregs
       argument (defaulting to 0). Update call to recog_for_combine_1.
       (combine_split_insns): Add old_nregs and new_nregs arguments,
       store the old and new max registers to them.
       (try_combine): Update calls to combine_split_insns and
       pass old_nregs and new_nregs for the i3 call to recog_for_combine.
       (find_split_point): Update call to combine_split_insns; ignoring
       the values there.
       (recog_for_combine_1): Add old_nregs and new_nregs arguments,
       if the insn was recognized (and not to no-op move), add the
       REG_DEAD notes to pnotes argument.
So not a complete review, but a concern.

What happens if the final pattern doesn't reference the newly created
pseduo?  Don't we end up with a REG_DEAD note anyway?  And are there
negative implications of having a REG_DEAD note for a pseudo that isn't
actually referenced in the pattern?

I could easily see splitters asking for new regs, then not needing them
in some cases.  Or multiple split attempts before hitting one that
matches.  In both cases ISTM like we can run into the situation noted above.

I had that concern too originally; I looked into it and found
distribute_notes is called with the new notes afterwards and will not
place the REG_DEAD note on an instruction if the register is no longer
referenced and place the REG_DEAD on the correct insn otherwise.
I'm not 100% convinced that it does what you're saying, but I haven't thrown it into a debugger to verify. So I'll go with your judgment since you're the one looking at it directly. You might consider a comment somewhere about this concern.

OK for the trunk.

jeff

Reply via email to