From patchwork Tue Apr 9 16:15:43 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Botcazou X-Patchwork-Id: 235139 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 E4F6A2C00AD for ; Wed, 10 Apr 2013 02:16:53 +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=JVeeYi2wqLtqKqN6+61ECCdjoqyb6p/rnvwwCHA19i1fVxKwTlzIu mq7ghjs6w4K/yZMHR+0rvEuVpBNGFuB4aBhCyUgDHz7kZvi3+H//H4CRGduBHuky fsr061+g4uWlZ6XKoXVGNzeTZMlvUrZLnRfm2frCJ35byJzC3zUy5c= 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=CgUleHeWmo0M5FHzzqpuL9BrxxE=; b=wd1EtKrDW4JI7Rt4M4a0sdd45C6p f7JQAdr+/UxH1rVkoeJv0OZqycb3Fh5tm4JHT1/WpVLHfueN51+IJ6rSFhkMoLUo Vwn5RX0lzmX0BGowgV6UnFqY9LvDTfp1AdaaGmu4M2qkgPK6GLPTpj+fpgDgBahB /RYonJBVSnuTMtU= Received: (qmail 8423 invoked by alias); 9 Apr 2013 16:16:46 -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 8412 invoked by uid 89); 9 Apr 2013 16:16:45 -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; Tue, 09 Apr 2013 16:16:44 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 7D517290039; Tue, 9 Apr 2013 18:16:42 +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 tzvxVAECaZ+m; Tue, 9 Apr 2013 18:16:42 +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 47051290007; Tue, 9 Apr 2013 18:16:42 +0200 (CEST) From: Eric Botcazou To: Jeff Law Cc: gcc-patches@gcc.gnu.org Subject: Re: Fill more delay slots in conditional returns Date: Tue, 09 Apr 2013 18:15:43 +0200 Message-ID: <2373459.H4ikUyDJrc@polaris> User-Agent: KMail/4.7.2 (Linux/3.1.10-1.19-desktop; KDE/4.7.2; x86_64; ; ) In-Reply-To: <515A49B4.3090700@redhat.com> References: <1769120.Os7kk4Zj9I@polaris> <2198501.fcZ53kPIeV@polaris> <515A49B4.3090700@redhat.com> MIME-Version: 1.0 X-Virus-Found: No > Ah, OK, yea, it makes perfect sense now. Approved. Thanks. > If you wanted to get ambitious, rewriting reorg.c to compute and use > dependency links to drive its candidate selection instead of the insane > tracking code it uses would be a huge step forward, both in terms of > cleanliness. It'd also help compile-time performance; we do a lot of > work to track resources that would be totally unnecessary. Yes, I agree that it's quite convoluted but, as Steven already said, rewriting it to use a more modern framework is hard because of SEQUENCEs and, frankly, I have neither the real incentive nor the time to start such an endeavor. Instead I can raise the following couple of points that I also ran into with the private port I'm working on: 1. When fill_simple_delay_slots is scanning backwards to find insns to put into slots, it calls update_reg_dead_note which collects REG_DEAD notes and puts them on the insn to be moved. But emit_delay_sequence will later drop them on the floor. I have one case where preserving such a REG_DEAD note is correct and makes a difference (slot filled vs empty). 2. In resource.c, when mark_target_live_regs is doing its liveness analysis, it extracts the insns wrapped in USEs by reorg.c: /* If this insn is a USE made by update_block, we care about the underlying insn. */ if (code == INSN && GET_CODE (PATTERN (insn)) == USE && INSN_P (XEXP (PATTERN (insn), 0))) real_insn = XEXP (PATTERN (insn), 0); and proceeds with the liveness analysis without further ado. Now, the story is different in find_dead_or_set_registers, which does: if (GET_CODE (PATTERN (insn)) == USE) { /* If INSN is a USE made by update_block, we care about the underlying insn. Any registers set by the underlying insn are live since the insn is being done somewhere else. */ if (INSN_P (XEXP (PATTERN (insn), 0))) mark_set_resources (XEXP (PATTERN (insn), 0), res, 0, MARK_SRC_DEST_CALL); and thus pessimizes the analysis by making registers unnecessary live. Again I have one case where not pessimizing is correct and makes a difference. Experimental patch attached, clean on the C testsuite for the private port. What do you think? Index: reorg.c =================================================================== --- reorg.c (revision 197617) +++ reorg.c (working copy) @@ -549,6 +549,11 @@ emit_delay_sequence (rtx insn, rtx list, remove_note (tem, note); break; + case REG_DEP_TRUE: + /* This was a REG_DEAD note that we want to preserve. */ + PUT_REG_NOTE_KIND (note, REG_DEAD); + break; + case REG_LABEL_OPERAND: case REG_LABEL_TARGET: /* Keep the label reference count up to date. */ @@ -1806,8 +1811,10 @@ update_reg_dead_notes (rtx insn, rtx del if (reg_referenced_p (XEXP (link, 0), PATTERN (insn))) { - /* Move the REG_DEAD note from P to INSN. */ + /* Move the REG_DEAD note from P to INSN but smuggle it so that + emit_delay_sequence doesn't drop it on the floor. */ remove_note (p, link); + PUT_REG_NOTE_KIND (link, REG_DEP_TRUE); XEXP (link, 1) = REG_NOTES (insn); REG_NOTES (insn) = link; } Index: resource.c =================================================================== --- resource.c (revision 197617) +++ resource.c (working copy) @@ -425,6 +425,7 @@ find_dead_or_set_registers (rtx target, for (insn = target; insn; insn = next) { rtx this_jump_insn = insn; + rtx real_insn = insn; next = NEXT_INSN (insn); @@ -443,7 +444,6 @@ find_dead_or_set_registers (rtx target, AND_COMPL_HARD_REG_SET (pending_dead_regs, needed.regs); AND_COMPL_HARD_REG_SET (res->regs, pending_dead_regs); CLEAR_HARD_REG_SET (pending_dead_regs); - continue; case BARRIER: @@ -454,14 +454,12 @@ find_dead_or_set_registers (rtx target, if (GET_CODE (PATTERN (insn)) == USE) { /* If INSN is a USE made by update_block, we care about the - underlying insn. Any registers set by the underlying insn - are live since the insn is being done somewhere else. */ + underlying insn. */ if (INSN_P (XEXP (PATTERN (insn), 0))) - mark_set_resources (XEXP (PATTERN (insn), 0), res, 0, - MARK_SRC_DEST_CALL); - - /* All other USE insns are to be ignored. */ - continue; + real_insn = XEXP (PATTERN (insn), 0); + else + /* All other USE insns are to be ignored. */ + continue; } else if (GET_CODE (PATTERN (insn)) == CLOBBER) continue; @@ -582,8 +580,8 @@ find_dead_or_set_registers (rtx target, } } - mark_referenced_resources (insn, &needed, true); - mark_set_resources (insn, &set, 0, MARK_SRC_DEST_CALL); + mark_referenced_resources (real_insn, &needed, true); + mark_set_resources (real_insn, &set, 0, MARK_SRC_DEST_CALL); COPY_HARD_REG_SET (scratch, set.regs); AND_COMPL_HARD_REG_SET (scratch, needed.regs);