On 2/19/25 9:13 PM, Andrew Pinski wrote:
So gcc.target/aarch64/rev16_2.c started to fail after r15-268-g9dbff9c05520a7,
the problem is combine now rejects the instruction combine. This happens because
after a different combine which uses a define_split and that define_split
creates
a new pseudo register. That new pseudo register is dead after the last
instruction
in the stream BUT combine never creates a REG_DEAD for it. So combine thinks it
is
still needed after and now with the i2 not changing, combine rejects the
combine.
Now combine should be creating a REG_DEAD for the new pseudo registers for the
last
instruction of the split. This fixes rev16_2.c and also improves the situtation
in
other cases by removing other dead instructions.
Bootstrapped and tested on aarch64-linux-gnu and x86_64-linux-gnu.
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.
jeff
