| Message ID | ZS7zE+uo4mdL6dr4@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | aarch64: Add new load/store pair fusion pass | expand |
On 10/17/23 14:48, Alex Coplan wrote: > haifa-sched.cc:remove_notes asserts that it lands on a real (non-note) > insn after advancing past NOTE_INSN_EPILOGUE_BEG, but with the upcoming > post-RA aarch64 load pair pass enabled, we can land on > NOTE_INSN_DELETED. > > This patch adjusts remove_notes to remove these if they occur at the > start of the epilogue instead of asserting. > > Bootstrapped/regtested as a series on aarch64-linux-gnu, OK for trunk? > > gcc/ChangeLog: > > * haifa-sched.cc (remove_notes): Allow for NOTE_INSN_DELETED at > the start of the epilgoue, remove these. One could argue that the pass should have actually deleted the insn rather than just turned it into a NOTE_INSN_DELETED. Is there some reason that's not done? A NOTE_INSN_DELETED carries no useful information. > + /* Skip over any NOTE_INSN_DELETED at the start of the epilogue. > + */ Don't bring the close comment down to a new line. If it fits, but it on the last line of the actual comment. Otherwise bring down part of comment so that we don't have the close comment on a line by itself. Jeff
Hi Jeff, On 19/10/2023 08:54, Jeff Law wrote: > > > On 10/17/23 14:48, Alex Coplan wrote: > > haifa-sched.cc:remove_notes asserts that it lands on a real (non-note) > > insn after advancing past NOTE_INSN_EPILOGUE_BEG, but with the upcoming > > post-RA aarch64 load pair pass enabled, we can land on > > NOTE_INSN_DELETED. > > > > This patch adjusts remove_notes to remove these if they occur at the > > start of the epilogue instead of asserting. > > > > Bootstrapped/regtested as a series on aarch64-linux-gnu, OK for trunk? > > > > gcc/ChangeLog: > > > > * haifa-sched.cc (remove_notes): Allow for NOTE_INSN_DELETED at > > the start of the epilgoue, remove these. > One could argue that the pass should have actually deleted the insn rather > than just turned it into a NOTE_INSN_DELETED. Is there some reason that's > not done? A NOTE_INSN_DELETED carries no useful information. Yeah, I think we can teach rtl-ssa to do that. I'm testing a patch to do so now. I think the pass is the first consumer of the deletion functionality (insn_change::DELETE) in rtl-ssa, and I can't see a reason for it to keep the NOTE_INSN_DELETED around. So consider the scheduler patch withdrawn for now, thanks! Alex > > > > + /* Skip over any NOTE_INSN_DELETED at the start of the epilogue. > > + */ > > Don't bring the close comment down to a new line. If it fits, but it on the > last line of the actual comment. Otherwise bring down part of comment so > that we don't have the close comment on a line by itself. > > Jeff
diff --git a/gcc/haifa-sched.cc b/gcc/haifa-sched.cc index 8e8add709b3..9f45528fbe9 100644 --- a/gcc/haifa-sched.cc +++ b/gcc/haifa-sched.cc @@ -4249,6 +4249,17 @@ remove_notes (rtx_insn *head, rtx_insn *tail) && NOTE_KIND (next) == NOTE_INSN_BASIC_BLOCK && next != next_tail) next = NEXT_INSN (next); + + /* Skip over any NOTE_INSN_DELETED at the start of the epilogue. + */ + while (NOTE_P (next) + && NOTE_KIND (next) == NOTE_INSN_DELETED) + { + auto tmp = NEXT_INSN (next); + delete_insn (next); + next = tmp; + } + gcc_assert (INSN_P (next)); add_reg_note (next, REG_SAVE_NOTE, GEN_INT (NOTE_INSN_EPILOGUE_BEG));