On Sun, Mar 16, 2025 at 10:10 AM Jeff Law <jeffreya...@gmail.com> wrote: > > > > 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.
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. Thanks, Andrew Pinski > > jeff
