Richard Sandiford writes:
> Senthil Kumar via Gcc-patches <gcc-patches@gcc.gnu.org> writes: >> Hi, >> >> I'm working on converting the AVR backend to MODE_CC, following >> the steps described for case #2 in the CC0 transition wiki page, >> and I've implemented the first three bullet >> points (https://github.com/saaadhu/gcc-avr-cc0/tree/avr-cc0-squashed). With >> the below patch, there are zero regressions (for mega and xmega >> subarchs) compared to the current mainline, as of yesterday. >> >> The wiki suggests using post-reload splitters, so that's the >> direction I took, but I ran into an issue where split_insn >> bails out early if RTX_FRAME_RELATED_P is true - this means >> that splits for REG_CC clobbering insns with >> RTX_FRAME_RELATED_P will never execute, resulting in a >> could-not-split insn ICE in the final stage. >> >> I see that the recog.c:peep2_attempt allows splitting of a >> RTX_FRAME_RELATED_P insn, provided the result of the split is a >> single insn. Would it be ok to modify try_split also to >> allow those kinds of insns (tentative patch attached, code >> copied over from peep2_attempt, only setting old and new_insn)? Or is there >> a different approach to fix this? > > I agree there's no obvious reason why splitting to a single insn > should be rejected but a peephole2 to a single instruction should be OK. > And reusing the existing, tried-and-tested code is the way to go. > > But could you split the code out of peep2_attempt into a subroutine > (probably still in recog.c) and reuse it in try_split? How does the below patch look? Bootstrapped and reg tested on x86_64-linux. > > BTW, just to check: is your email address in MAINTAINERS still correct? It was out-of-date, yes - updated now. Regards Senthil 2020-08-13 Senthil Kumar Selvaraj <saaa...@gcc.gnu.org> gcc/ChangeLog: * emit-rtl.c (try_split): Call copy_frame_info_to_split_insn to split certain RTX_FRAME_RELATED_P insns. * recog.c (copy_frame_info_to_split_insn): New function. (peep2_attempt): Split copying of frame related info of RTX_FRAME_RELATED_P insns into above function and call it. * recog.h (copy_frame_info_to_split_insn): Declare it. diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c index f9b0e9714d9..3706f0a03fd 100644 --- a/gcc/emit-rtl.c +++ b/gcc/emit-rtl.c @@ -3822,10 +3822,6 @@ try_split (rtx pat, rtx_insn *trial, int last) int njumps = 0; rtx_insn *call_insn = NULL; - /* We're not good at redistributing frame information. */ - if (RTX_FRAME_RELATED_P (trial)) - return trial; - if (any_condjump_p (trial) && (note = find_reg_note (trial, REG_BR_PROB, 0))) split_branch_probability @@ -3842,6 +3838,7 @@ try_split (rtx pat, rtx_insn *trial, int last) if (!seq) return trial; + int split_insn_count = 0; /* Avoid infinite loop if any insn of the result matches the original pattern. */ insn_last = seq; @@ -3850,11 +3847,25 @@ try_split (rtx pat, rtx_insn *trial, int last) if (INSN_P (insn_last) && rtx_equal_p (PATTERN (insn_last), pat)) return trial; + split_insn_count++; if (!NEXT_INSN (insn_last)) break; insn_last = NEXT_INSN (insn_last); } + /* We're not good at redistributing frame information if + the split occurs before reload or if it results in more + than one insn. */ + if (RTX_FRAME_RELATED_P (trial)) + { + if (!reload_completed || split_insn_count != 1) + return trial; + + rtx_insn *new_insn = seq; + rtx_insn *old_insn = trial; + copy_frame_info_to_split_insn (old_insn, new_insn); + } + /* We will be adding the new sequence to the function. The splitters may have introduced invalid RTL sharing, so unshare the sequence now. */ unshare_all_rtl_in_chain (seq); diff --git a/gcc/recog.c b/gcc/recog.c index 25f19b1b1cf..e024597f9d7 100644 --- a/gcc/recog.c +++ b/gcc/recog.c @@ -3277,6 +3277,78 @@ peep2_reinit_state (regset live) COPY_REG_SET (peep2_insn_data[MAX_INSNS_PER_PEEP2].live_before, live); } +/* Copies frame related info of an insn (old_insn) to the single + insn (new_insn) that was obtained by splitting old_insn. */ + +void +copy_frame_info_to_split_insn (rtx_insn *old_insn, rtx_insn *new_insn) +{ + bool any_note = false; + rtx note; + + if (!RTX_FRAME_RELATED_P (old_insn)) + return; + + RTX_FRAME_RELATED_P (new_insn) = 1; + + /* Allow the backend to fill in a note during the split. */ + for (note = REG_NOTES (new_insn); note ; note = XEXP (note, 1)) + switch (REG_NOTE_KIND (note)) + { + case REG_FRAME_RELATED_EXPR: + case REG_CFA_DEF_CFA: + case REG_CFA_ADJUST_CFA: + case REG_CFA_OFFSET: + case REG_CFA_REGISTER: + case REG_CFA_EXPRESSION: + case REG_CFA_RESTORE: + case REG_CFA_SET_VDRAP: + any_note = true; + break; + default: + break; + } + + /* If the backend didn't supply a note, copy one over. */ + if (!any_note) + for (note = REG_NOTES (old_insn); note ; note = XEXP (note, 1)) + switch (REG_NOTE_KIND (note)) + { + case REG_FRAME_RELATED_EXPR: + case REG_CFA_DEF_CFA: + case REG_CFA_ADJUST_CFA: + case REG_CFA_OFFSET: + case REG_CFA_REGISTER: + case REG_CFA_EXPRESSION: + case REG_CFA_RESTORE: + case REG_CFA_SET_VDRAP: + add_reg_note (new_insn, REG_NOTE_KIND (note), XEXP (note, 0)); + any_note = true; + break; + default: + break; + } + + /* If there still isn't a note, make sure the unwind info sees the + same expression as before the split. */ + if (!any_note) + { + rtx old_set, new_set; + + /* The old insn had better have been simple, or annotated. */ + old_set = single_set (old_insn); + gcc_assert (old_set != NULL); + + new_set = single_set (new_insn); + if (!new_set || !rtx_equal_p (new_set, old_set)) + add_reg_note (new_insn, REG_FRAME_RELATED_EXPR, old_set); + } + + /* Copy prologue/epilogue status. This is required in order to keep + proper placement of EPILOGUE_BEG and the DW_CFA_remember_state. */ + maybe_copy_prologue_epilogue_insn (old_insn, new_insn); +} + /* While scanning basic block BB, we found a match of length MATCH_LEN, starting at INSN. Perform the replacement, removing the old insns and replacing them with ATTEMPT. Returns the last insn emitted, or NULL @@ -3297,9 +3369,6 @@ peep2_attempt (basic_block bb, rtx_insn *insn, int match_len, rtx_insn *attempt) old_insn = peep2_insn_data[peep2_current].insn; if (RTX_FRAME_RELATED_P (old_insn)) { - bool any_note = false; - rtx note; - if (match_len != 0) return NULL; @@ -3313,64 +3382,7 @@ peep2_attempt (basic_block bb, rtx_insn *insn, int match_len, rtx_insn *attempt) return NULL; /* We have a 1-1 replacement. Copy over any frame-related info. */ - RTX_FRAME_RELATED_P (new_insn) = 1; - - /* Allow the backend to fill in a note during the split. */ - for (note = REG_NOTES (new_insn); note ; note = XEXP (note, 1)) - switch (REG_NOTE_KIND (note)) - { - case REG_FRAME_RELATED_EXPR: - case REG_CFA_DEF_CFA: - case REG_CFA_ADJUST_CFA: - case REG_CFA_OFFSET: - case REG_CFA_REGISTER: - case REG_CFA_EXPRESSION: - case REG_CFA_RESTORE: - case REG_CFA_SET_VDRAP: - any_note = true; - break; - default: - break; - } - - /* If the backend didn't supply a note, copy one over. */ - if (!any_note) - for (note = REG_NOTES (old_insn); note ; note = XEXP (note, 1)) - switch (REG_NOTE_KIND (note)) - { - case REG_FRAME_RELATED_EXPR: - case REG_CFA_DEF_CFA: - case REG_CFA_ADJUST_CFA: - case REG_CFA_OFFSET: - case REG_CFA_REGISTER: - case REG_CFA_EXPRESSION: - case REG_CFA_RESTORE: - case REG_CFA_SET_VDRAP: - add_reg_note (new_insn, REG_NOTE_KIND (note), XEXP (note, 0)); - any_note = true; - break; - default: - break; - } - - /* If there still isn't a note, make sure the unwind info sees the - same expression as before the split. */ - if (!any_note) - { - rtx old_set, new_set; - - /* The old insn had better have been simple, or annotated. */ - old_set = single_set (old_insn); - gcc_assert (old_set != NULL); - - new_set = single_set (new_insn); - if (!new_set || !rtx_equal_p (new_set, old_set)) - add_reg_note (new_insn, REG_FRAME_RELATED_EXPR, old_set); - } - - /* Copy prologue/epilogue status. This is required in order to keep - proper placement of EPILOGUE_BEG and the DW_CFA_remember_state. */ - maybe_copy_prologue_epilogue_insn (old_insn, new_insn); + copy_frame_info_to_split_insn (old_insn, new_insn); } /* If we are splitting a CALL_INSN, look for the CALL_INSN diff --git a/gcc/recog.h b/gcc/recog.h index 3e4b55bdf3f..ae3675f5c82 100644 --- a/gcc/recog.h +++ b/gcc/recog.h @@ -150,6 +150,8 @@ extern rtx_insn *peephole2_insns (rtx, rtx_insn *, int *); extern int store_data_bypass_p (rtx_insn *, rtx_insn *); extern int if_test_bypass_p (rtx_insn *, rtx_insn *); +extern void copy_frame_info_to_split_insn (rtx_insn *, rtx_insn *); + #ifndef GENERATOR_FILE /* Try recognizing the instruction INSN, and return the code number that results.
