From patchwork Sun Apr 14 09:43:22 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Botcazou X-Patchwork-Id: 236423 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 87EE12C00AB for ; Sun, 14 Apr 2013 19:46:45 +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:from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; q=dns; s= default; b=OWj9b6fSeDgZJ7Lbv9F0am6GiCx7HWpoRyqRYFJ/8IqCztsloaTY9 WN+S7acUnEK6fSz9gu5Q6D4rTRJOTjsjMrSz4uhKgjPb6ZsBxOVr+7kQTjVUHnel j6QgS7OlNfMqNrKAnDM7Ufxj+dh1lVahe6xURE77G7ZqE8+SVnqcHw= 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:from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; s=default; bh=mtQE+Sr+FmDj0WS5dxqpq+r9prc=; b=baiFCHeRh51WKvrFYctQEqZnjqv/ Ldujhm7iqsdpvYGCRKQ7w6M273pANWN/mMRPzULJnuUIlZqtsMYqC7YVEM+4M2Qd zW/SVypKzHhb/fUCQuNQ6chhBTklCIfM57wwoLi7uod4gDgU1XqI7JC07p3cL1Js hTl5b+Xol1q8kto= Received: (qmail 15721 invoked by alias); 14 Apr 2013 09:46:37 -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 15710 invoked by uid 89); 14 Apr 2013 09:46:37 -0000 X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL, BAYES_00, KHOP_THREADED autolearn=ham version=3.3.1 Received: from mel.act-europe.fr (HELO mel.act-europe.fr) (194.98.77.210) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Sun, 14 Apr 2013 09:46:36 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 96D4229001F; Sun, 14 Apr 2013 11:46:34 +0200 (CEST) Received: from mel.act-europe.fr ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id JiwI6bPnKiwC; Sun, 14 Apr 2013 11:46:34 +0200 (CEST) Received: from polaris.localnet (bon31-6-88-161-99-133.fbx.proxad.net [88.161.99.133]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mel.act-europe.fr (Postfix) with ESMTP id 5A69C29000E; Sun, 14 Apr 2013 11:46:34 +0200 (CEST) From: Eric Botcazou To: Jeff Law Cc: gcc-patches@gcc.gnu.org Subject: Re: Fill more delay slots in conditional returns Date: Sun, 14 Apr 2013 11:43:22 +0200 Message-ID: <5082198.kTrRiqZjhC@polaris> User-Agent: KMail/4.7.2 (Linux/3.1.10-1.19-desktop; KDE/4.7.2; x86_64; ; ) In-Reply-To: <51645569.1090501@redhat.com> References: <1769120.Os7kk4Zj9I@polaris> <2373459.H4ikUyDJrc@polaris> <51645569.1090501@redhat.com> MIME-Version: 1.0 X-Virus-Found: No > I don't recall ever working on this aspect of reorg. The obvious worry > is that with reorg moving stuff around those notes may not be valid > anymore in the general case. Yes, in the general case I agree that's too dangerous. In this particular case, i.e. backward scan only, this might be plausible, although one has probably to worry about what happens if the insn is removed from the delay slot and put back into the RTL stream. > Just a note, there's a formatting error in this code. The real_insn = > statement is indented too far. Feel free to fix that :-) Patch attached (I've also reindented a block of code in reorg.c), tested on the SPARC and applied on the mainline. > Seems reasonable. This might just be an oversight by the original > author. Effectively we're going to start marking the referenced > resources with your patch. We still defer killing as far as I can tell. OK. > If you want to run with it, I won't object. My worry would be we might > not see the fallout for a long time as the number of things that have to > fall into place for an observable failure here are very high. Indeed, I spotted these two cases when I was playing with the allocation of registers for the private target. They showed up only under very specific circumstances; with the default setting, neither makes a difference for the various testcases I'm looking at. So, in the end, I probably won't pursue. Thanks for your feedback. * reorg.c (fill_simple_delay_slots): Reindent block of code. * resource.c (mark_target_live_regs): Reformat conditional block. Index: reorg.c =================================================================== --- reorg.c (revision 197926) +++ reorg.c (working copy) @@ -2144,69 +2144,66 @@ fill_simple_delay_slots (int non_jumps_p if (CALL_P (insn)) maybe_never = 1; - for (trial = next_nonnote_insn (insn); !stop_search_p (trial, 1); - trial = next_trial) - { - next_trial = next_nonnote_insn (trial); - - /* This must be an INSN or CALL_INSN. */ - pat = PATTERN (trial); - - /* Stand-alone USE and CLOBBER are just for flow. */ - if (GET_CODE (pat) == USE || GET_CODE (pat) == CLOBBER) - continue; - - /* If this already has filled delay slots, get the insn needing - the delay slots. */ - if (GET_CODE (pat) == SEQUENCE) - trial_delay = XVECEXP (pat, 0, 0); - else - trial_delay = trial; - - /* Stop our search when seeing a jump. */ - if (JUMP_P (trial_delay)) - break; - - /* See if we have a resource problem before we try to - split. */ - if (GET_CODE (pat) != SEQUENCE - && ! insn_references_resource_p (trial, &set, true) - && ! insn_sets_resource_p (trial, &set, true) - && ! insn_sets_resource_p (trial, &needed, true) + for (trial = next_nonnote_insn (insn); !stop_search_p (trial, 1); + trial = next_trial) + { + next_trial = next_nonnote_insn (trial); + + /* This must be an INSN or CALL_INSN. */ + pat = PATTERN (trial); + + /* Stand-alone USE and CLOBBER are just for flow. */ + if (GET_CODE (pat) == USE || GET_CODE (pat) == CLOBBER) + continue; + + /* If this already has filled delay slots, get the insn needing + the delay slots. */ + if (GET_CODE (pat) == SEQUENCE) + trial_delay = XVECEXP (pat, 0, 0); + else + trial_delay = trial; + + /* Stop our search when seeing a jump. */ + if (JUMP_P (trial_delay)) + break; + + /* See if we have a resource problem before we try to split. */ + if (GET_CODE (pat) != SEQUENCE + && ! insn_references_resource_p (trial, &set, true) + && ! insn_sets_resource_p (trial, &set, true) + && ! insn_sets_resource_p (trial, &needed, true) #ifdef HAVE_cc0 - && ! (reg_mentioned_p (cc0_rtx, pat) && ! sets_cc0_p (pat)) + && ! (reg_mentioned_p (cc0_rtx, pat) && ! sets_cc0_p (pat)) #endif - && ! (maybe_never && may_trap_or_fault_p (pat)) - && (trial = try_split (pat, trial, 0)) - && eligible_for_delay (insn, slots_filled, trial, flags) - && ! can_throw_internal(trial)) - { - next_trial = next_nonnote_insn (trial); - delay_list = add_to_delay_list (trial, delay_list); - + && ! (maybe_never && may_trap_or_fault_p (pat)) + && (trial = try_split (pat, trial, 0)) + && eligible_for_delay (insn, slots_filled, trial, flags) + && ! can_throw_internal(trial)) + { + next_trial = next_nonnote_insn (trial); + delay_list = add_to_delay_list (trial, delay_list); #ifdef HAVE_cc0 - if (reg_mentioned_p (cc0_rtx, pat)) - link_cc0_insns (trial); + if (reg_mentioned_p (cc0_rtx, pat)) + link_cc0_insns (trial); #endif + delete_related_insns (trial); + if (slots_to_fill == ++slots_filled) + break; + continue; + } + + mark_set_resources (trial, &set, 0, MARK_SRC_DEST_CALL); + mark_referenced_resources (trial, &needed, true); - delete_related_insns (trial); - if (slots_to_fill == ++slots_filled) - break; - continue; - } - - mark_set_resources (trial, &set, 0, MARK_SRC_DEST_CALL); - mark_referenced_resources (trial, &needed, true); - - /* Ensure we don't put insns between the setting of cc and the - comparison by moving a setting of cc into an earlier delay - slot since these insns could clobber the condition code. */ - set.cc = 1; - - /* If this is a call, we might not get here. */ - if (CALL_P (trial_delay)) - maybe_never = 1; - } + /* Ensure we don't put insns between the setting of cc and the + comparison by moving a setting of cc into an earlier delay + slot since these insns could clobber the condition code. */ + set.cc = 1; + + /* If this is a call, we might not get here. */ + if (CALL_P (trial_delay)) + maybe_never = 1; + } /* If there are slots left to fill and our search was stopped by an unconditional branch, try the insn at the branch target. We can Index: resource.c =================================================================== --- resource.c (revision 197926) +++ resource.c (working copy) @@ -990,9 +990,10 @@ mark_target_live_regs (rtx insns, rtx ta /* If this insn is a USE made by update_block, we care about the underlying insn. */ - if (code == INSN && GET_CODE (PATTERN (insn)) == USE + if (code == INSN + && GET_CODE (PATTERN (insn)) == USE && INSN_P (XEXP (PATTERN (insn), 0))) - real_insn = XEXP (PATTERN (insn), 0); + real_insn = XEXP (PATTERN (insn), 0); if (CALL_P (real_insn)) {