From patchwork Sat Apr 13 00:02:21 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steven Bosscher X-Patchwork-Id: 236284 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 550282C00A5 for ; Sat, 13 Apr 2013 10:03:11 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:from:date:message-id:subject:to:cc:content-type; q=dns; s=default; b=yHN94Z99cbF/CqzxNY4Kbszl8hr/hyodDz+d3n9nNZ3 bFFe/+d9uibCbTh4SFSvX8hMynneiX1FuuGXqvbanx2yJ+hXlWD76Zcj48hKaBcc XPEzDlKsOEGMtpH62xs7YBnj/1eUBFdf55gm3KnjlHTMlLbU1Mxi0lso/2SG8IJM = DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:from:date:message-id:subject:to:cc:content-type; s=default; bh=qHupkIWU++kQZ+WrfQ8iZSUHpTM=; b=lg0phidwgLjWe1tYm uYD+MlEf+iJVJcPjt/vgW7Vv4BlLEf/Nbn9qk1BUSMUt1FUc/Svo6LOwdgsvJvsr h4Rhin6MLSihhzl7A7Q7ibDTtdF4vFhhlNswq33Sb8EkJ/X8Pw5S+S/zJ3qHImRj 57B1z02vLB90krZdCZYNwwTBs0= Received: (qmail 9490 invoked by alias); 13 Apr 2013 00:03:05 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 9481 invoked by uid 89); 13 Apr 2013 00:03:05 -0000 X-Spam-SWARE-Status: No, score=-4.4 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, KHOP_RCVD_TRUST, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE autolearn=ham version=3.3.1 Received: from mail-ve0-f179.google.com (HELO mail-ve0-f179.google.com) (209.85.128.179) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Sat, 13 Apr 2013 00:03:03 +0000 Received: by mail-ve0-f179.google.com with SMTP id cz11so2834197veb.24 for ; Fri, 12 Apr 2013 17:03:02 -0700 (PDT) X-Received: by 10.220.203.130 with SMTP id fi2mr10066875vcb.52.1365811382138; Fri, 12 Apr 2013 17:03:02 -0700 (PDT) MIME-Version: 1.0 Received: by 10.58.240.13 with HTTP; Fri, 12 Apr 2013 17:02:21 -0700 (PDT) From: Steven Bosscher Date: Sat, 13 Apr 2013 02:02:21 +0200 Message-ID: Subject: [patch][DF] do not call df_insn_delete in remove_insn, only unlink the insn To: GCC Patches Cc: Paolo Bonzini X-Virus-Found: No 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); }