Hello, emit-rtl.c:remove_insn calls df_insn_delete on insns that have not been emitted to the insn chain, and therefore don't have DF cache entries yet. I think it's wrong for remove_insn to call df_insn_delete.
delete_insn should be used to completely destroy an insn and all associated data including DF caches. Calling remove_insn to really delete an insn is also wrong because LABEL_NUSES isn't updated. Fortunately, almost all code already uses delete_insn so this patch is small. remove_insn should only unlink an insn from the current insns chain (the function body or an open start_sequence chain). The attached patch makes it so... In fact, without this patch, sel-sched-ir.c had to work around this problem (remove_insn doing df_insn_delete) but it was leaking DF caches as a result. Probably there are other places that can be simplified now to use remove_insn instead of hacking the insn chain by hand. I've looked at all remove_insn and delete_insn callers to make sure this patch does In general I think it's wrong for emit-rtl to have any dependence on DF (DF depends on RTL, but not the other way around) but that's not something I want to work on right now. Bootstrapped&tested on x86_64-unknown-linux-gnu. OK? Ciao! Steven
* emit-rtl.c (remove_insn): Do not call df_insn_delete here. * cfgrtl.c (delete_insn): Call it here instead. * lra-spills.c (lra_final_code_change): Use delete_insn. * haifa-sched.c (sched_remove_insn): Likewise. * sel-sched-ir.c (return_nop_to_pool): Clear INSN_DELETED_P for nops returning to the nop pool. (sel_remove_insn): Simplify the only_disconnect case via remove_insn, use delete_insn for definitive removal. Clear BLOCK_FOR_INSN. Index: emit-rtl.c =================================================================== --- emit-rtl.c (revision 197536) +++ emit-rtl.c (working copy) @@ -3908,8 +3908,21 @@ set_insn_deleted (rtx insn) } -/* Remove an insn from its doubly-linked list. This function knows how - to handle sequences. */ +/* Unlink INSN from the insn chain. + + This function knows how to handle sequences. + + This function does not invalidate data flow information associated with + INSN (i.e. does not call df_insn_delete). That makes this function + usable for only disconnecting an insn from the chain, and re-emit it + elsewhere later. + + To later insert INSN elsewhere in the insn chain via add_insn and + similar functions, PREV_INSN and NEXT_INSN must be nullified by + the caller. Nullifying them here breaks many insn chain walks. + + To really delete an insn and related DF information, use delete_insn. */ + void remove_insn (rtx insn) { @@ -3968,10 +3981,6 @@ remove_insn (rtx insn) gcc_assert (stack); } - /* Invalidate data flow information associated with INSN. */ - if (INSN_P (insn)) - df_insn_delete (insn); - /* Fix up basic block boundaries, if necessary. */ if (!BARRIER_P (insn) && (bb = BLOCK_FOR_INSN (insn))) Index: cfgrtl.c =================================================================== --- cfgrtl.c (revision 197536) +++ cfgrtl.c (working copy) @@ -164,6 +164,8 @@ delete_insn (rtx insn) { /* If this insn has already been deleted, something is very wrong. */ gcc_assert (!INSN_DELETED_P (insn)); + if (INSN_P (insn)) + df_insn_delete (insn); remove_insn (insn); INSN_DELETED_P (insn) = 1; } Index: lra-spills.c =================================================================== --- lra-spills.c (revision 197536) +++ lra-spills.c (working copy) @@ -639,7 +639,7 @@ lra_final_code_change (void) need them anymore and don't want to waste compiler time processing them in a few subsequent passes. */ lra_invalidate_insn_data (insn); - remove_insn (insn); + delete_insn (insn); continue; } Index: haifa-sched.c =================================================================== --- haifa-sched.c (revision 197536) +++ haifa-sched.c (working copy) @@ -8198,7 +8198,7 @@ sched_remove_insn (rtx insn) change_queue_index (insn, QUEUE_NOWHERE); current_sched_info->add_remove_insn (insn, 1); - remove_insn (insn); + delete_insn (insn); } /* Clear priorities of all instructions, that are forward dependent on INSN. Index: sel-sched-ir.c =================================================================== --- sel-sched-ir.c (revision 197536) +++ sel-sched-ir.c (working copy) @@ -1065,6 +1065,9 @@ return_nop_to_pool (insn_t nop, bool ful gcc_assert (INSN_IN_STREAM_P (nop)); sel_remove_insn (nop, false, full_tidying); + /* We'll recycle this nop. */ + INSN_DELETED_P (nop) = 0; + if (nop_pool.n == nop_pool.s) nop_pool.v = XRESIZEVEC (rtx, nop_pool.v, (nop_pool.s = 2 * nop_pool.s + 1)); @@ -3929,31 +3932,19 @@ sel_remove_insn (insn_t insn, bool only_ } if (only_disconnect) - { - insn_t prev = PREV_INSN (insn); - insn_t next = NEXT_INSN (insn); - basic_block bb = BLOCK_FOR_INSN (insn); - - NEXT_INSN (prev) = next; - PREV_INSN (next) = prev; - - if (BB_HEAD (bb) == insn) - { - gcc_assert (BLOCK_FOR_INSN (prev) == bb); - BB_HEAD (bb) = prev; - } - if (BB_END (bb) == insn) - BB_END (bb) = prev; - } + remove_insn (insn); else { - remove_insn (insn); + delete_insn (insn); clear_expr (INSN_EXPR (insn)); } - /* It is necessary to null this fields before calling add_insn (). */ + /* It is necessary to NULL these fields in case we are going to re-insert + INSN into the insns stream, as will usually happen in the ONLY_DISCONNECT + case, but also for NOPs that we will return to the nop pool. */ PREV_INSN (insn) = NULL_RTX; NEXT_INSN (insn) = NULL_RTX; + set_block_for_insn (insn, NULL); return tidy_control_flow (bb, full_tidying); }