From patchwork Fri Oct 19 15:22:58 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joern Rennecke X-Patchwork-Id: 192735 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]) by ozlabs.org (Postfix) with SMTP id 6FFAA2C0079 for ; Sat, 20 Oct 2012 02:23:12 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1351264992; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Message-ID:Date:From:To:Cc:Subject:References:In-Reply-To: MIME-Version:Content-Type:Content-Transfer-Encoding:User-Agent: Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:Sender:Delivered-To; bh=zmmaXums4HeAmQK3I63R zidRS00=; b=f471grIyX4c0v3nzgn96L+HdHP+loXjM8r6cvZ4niAIemv0Fvp7z 0wDqdYEh5pGpOcxVvB779ZxdgV9U7acLDtrKj7M4vMwrMRUQfOGr1L7dLui1JZgh 0yPNLAfZdPo5WMFymkGrSLCovWC05c6+/1mWUrl5e4zgsptRQNFw2O0= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Message-ID:Date:From:To:Cc:Subject:References:In-Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding:User-Agent:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=PGfbFluY5ZSLUxt36XL13e+rj5ApwixTS+p0zyovokp87Oq/699opCmXTtHNM5 C4dQhxOny55DXQOZJtxP9m37gSUvHzemRkb/AoOWvXmHov2z4XjsSVTaQgTzbiHW sIChqrtYwWCQX26v1RM3ViwWoTiAkgIgWgvGKRfxMaRX0=; Received: (qmail 23997 invoked by alias); 19 Oct 2012 15:23:06 -0000 Received: (qmail 23987 invoked by uid 22791); 19 Oct 2012 15:23:05 -0000 X-SWARE-Spam-Status: No, hits=-2.7 required=5.0 tests=AWL, BAYES_00, KHOP_THREADED, MIME_QP_LONG_LINE, RCVD_IN_HOSTKARMA_NO, TW_BJ X-Spam-Check-By: sourceware.org Received: from c62.cesmail.net (HELO c62.cesmail.net) (216.154.195.54) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 19 Oct 2012 15:23:00 +0000 Received: from unknown (HELO delta2) ([192.168.1.50]) by c62.cesmail.net with ESMTP; 19 Oct 2012 11:22:58 -0400 Received: from cust213-dsl91-135-11.idnet.net (cust213-dsl91-135-11.idnet.net [91.135.11.213]) by webmail.spamcop.net (Horde MIME library) with HTTP; Fri, 19 Oct 2012 11:22:58 -0400 Message-ID: <20121019112258.f139lurwg0g8w8gk-nzlynne@webmail.spamcop.net> Date: Fri, 19 Oct 2012 11:22:58 -0400 From: Joern Rennecke To: Richard Sandiford Cc: gcc-patches@gcc.gnu.org, Eric Botcazou Subject: RFA: fix dbr_schedule to leave unique ids unique References: <20121015013935.v6k1ge59foks8s88-nzlynne@webmail.spamcop.net> <87391flcjy.fsf@talisman.home> <20121016153515.0nmsft8o2soo4s40-nzlynne@webmail.spamcop.net> <87lif3k246.fsf@talisman.home> <20121018162901.49c9o85e00gswks0-nzlynne@webmail.spamcop.net> <87haprjxx6.fsf@talisman.home> In-Reply-To: <87haprjxx6.fsf@talisman.home> MIME-Version: 1.0 User-Agent: Internet Messaging Program (IMP) H3 (4.1.4) 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 Quoting Richard Sandiford : > Joern Rennecke writes: >> Quoting Richard Sandiford : >>> The fact that we even have shared unique ids is pretty bad -- and surely >>> a contradiction in terms -- but I think both ways of handling them rely >>> on the length being the same for all copies. If we don't record a length >>> (your version), we won't set something_changed if the length of this copy >>> did in fact change, which could lead to branches remaining too short. >> >> That is not the case, as the current length of the inner insn is added to >> new_length (of the outer insn). >> >>> If we do record a length (current version), >> >> In the current version, the length of the inner insn is calculated anew >> each iteration, so only the out-of-sequence copy suffers from the bogosity. >> >>> we could end up changing >>> the length of previous copies that have already been processed in >>> this iteration. >> >> Both that, and also using the length or the previous insn for the inner >> length calculation, and ratcheting them up together. >> In fact, this can make delay slot instructions ineligible for the delay slot >> they are in. Also, the unwanted length cross-interference can violate >> alignment invariants, and this leads to invalid code on ARCompact. > > But if you're saying that ARCompat wants different copies of the same insn > (with the same uid) to have different lengths, then please fix dbr_schedule > so that it uses different uids. The "i == 0" thing is just too hackish IMO. Implemented with the attached patch. I've been wondering if I should use copy_insn, but that doesn't seem to make any real difference after reload. Also, copying only PATTERN, INSN_LOCATION, and REG_NOTES into the new insn obtained from make_insn_raw had seemed a possibility, but there is no firm assurance that we will only see insn with the code INSN. In the end, continuing to use copy_rtx to make the copy seems least likely to break something. And now that the copying is in one place, it's easier to change the way it is done if you want to try that. We waste a bit of memory (temporarily, as it is garbage collected) by using make_insn_raw just to increment cur_insn_uid. Alternatives would be to move the function inside emit-rtl.c, or have emit-rtl.h provide a means to increment cur_insn_uid. Well, or we could directly access crtl->emit.x_cur_insn_uid, but that would certainly be hackish. regression tested on i686-pc-linux-gnu X cris-elf (cris-elf has delayed branches, CASE_VECTOR_SHORTEN_MODE, is not currently affected by PR54938, and builds libgfortran.) C / C++ / objc regression tests for mipsel-elf also passed. 2012-10-19 Joern Rennecke * reorg.c (copy_delay_slot_insn): New function. (steal_delay_list_from_target, fill_slots_from_thread): Use it. (fill_simple_delay_slots): Likewise. Index: reorg.c =================================================================== --- reorg.c (revision 192573) +++ reorg.c (working copy) @@ -1179,6 +1179,19 @@ check_annul_list_true_false (int annul_t return 1; } +/* Copy INSN, which is to remain in place (in fill_slots_from_thread parlance, + it is from a thread we don't own), for use in a delay slot. */ +static rtx +copy_delay_slot_insn (rtx insn) +{ + /* Copy INSN with its rtx_code, all its notes, location etc. */ + insn = copy_rtx (insn); + /* Get new UID. */ + rtx new_insn = make_insn_raw (PATTERN (insn)); + INSN_UID (insn) = INSN_UID (new_insn); + return insn; +} + /* INSN branches to an insn whose pattern SEQ is a SEQUENCE. Given that the condition tested by INSN is CONDITION and the resources shown in OTHER_NEEDED are needed after INSN, see whether INSN can take all the insns @@ -1297,7 +1310,7 @@ steal_delay_list_from_target (rtx insn, { if (must_annul) used_annul = 1; - temp = copy_rtx (trial); + temp = copy_delay_slot_insn (trial); INSN_FROM_TARGET_P (temp) = 1; new_delay_list = add_to_delay_list (temp, new_delay_list); total_slots_filled++; @@ -2369,7 +2382,8 @@ fill_simple_delay_slots (int non_jumps_p if (new_label) { delay_list - = add_to_delay_list (copy_rtx (next_trial), delay_list); + = add_to_delay_list (copy_delay_slot_insn (next_trial), + delay_list); slots_filled++; reorg_redirect_jump (trial, new_label); @@ -2793,7 +2807,7 @@ fill_slots_from_thread (rtx insn, rtx co else new_thread = next_active_insn (trial); - temp = own_thread ? trial : copy_rtx (trial); + temp = own_thread ? trial : copy_delay_slot_insn (trial); if (thread_if_true) INSN_FROM_TARGET_P (temp) = 1; @@ -2974,7 +2988,7 @@ fill_slots_from_thread (rtx insn, rtx co else new_thread = next_active_insn (trial); - ninsn = own_thread ? trial : copy_rtx (trial); + ninsn = own_thread ? trial : copy_delay_slot_insn (trial); if (thread_if_true) INSN_FROM_TARGET_P (ninsn) = 1;