From patchwork Fri Mar 28 10:20:00 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Botcazou X-Patchwork-Id: 334643 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.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id B59AE14008C for ; Fri, 28 Mar 2014 21:22:17 +1100 (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=l4dt044jlXo8ycyfQku3Y6zTBryZWVePquNWjV3W1W0NO5wk9KXGN aSTujNeGJIdLVKbzmlzVurEiIUA/i9wxwr8irnmMuMS+lrRZD38ZgYd7rNgFHejg W/Iv5cwAd5SImdZa+5pLQDenk2xAOYhh3ZDCdj4m4MeX0t/hvsw6Bk= 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=MdgyrLeWhPyZOxYzi/a+af2J+zw=; b=iw4sf5L8dqh1EW38SoCcGnWrgqtm vDMxnkB/+nbGQGESLsRmmrPTQxUPaNmUfGxvp4cTS6b0N6+iIFPTWxQ65ETu1P35 udaT2qyZpQ4dCOstTCxKtDGEDYSNHvSGV7hYBF4HokxYJ0ePO/2XQYsmDHctX/1F Gx8rw8kGfcBaVxg= Received: (qmail 30857 invoked by alias); 28 Mar 2014 10:22:10 -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 30845 invoked by uid 89); 28 Mar 2014 10:22:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00 autolearn=ham version=3.3.2 X-HELO: smtp.eu.adacore.com Received: from mel.act-europe.fr (HELO smtp.eu.adacore.com) (194.98.77.210) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 28 Mar 2014 10:22:08 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 47A09270D9CE; Fri, 28 Mar 2014 11:22:05 +0100 (CET) Received: from smtp.eu.adacore.com ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id pJ8pq3NAyk7d; Fri, 28 Mar 2014 11:22:05 +0100 (CET) Received: from polaris.localnet (bon31-6-88-161-99-133.fbx.proxad.net [88.161.99.133]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp.eu.adacore.com (Postfix) with ESMTPSA id 07CB4270D95A; Fri, 28 Mar 2014 11:22:04 +0100 (CET) From: Eric Botcazou To: Joern Rennecke Cc: gcc-patches@gcc.gnu.org Subject: Re: RFA: Fix PR rtl-optimization/60651 Date: Fri, 28 Mar 2014 11:20 +0100 Message-ID: <1483548.tKn4spAyxv@polaris> User-Agent: KMail/4.7.2 (Linux/3.1.10-1.29-desktop; KDE/4.7.2; x86_64; ; ) In-Reply-To: References: <4521715.uGsIsYq7k2@polaris> MIME-Version: 1.0 > However, the first call is for blocks with incoming abnormal edges. > If these are empty, the change as I wrote it yesterday is fine, but not > when they are non-empty; in that case, we should indeed insert before the > first instruction in that block. OK, so the issue is specific to empty basic blocks and boils down to inserting instructions in a FIFO manner into them. > This can be archived by finding an insert-before position using NEXT_INSN > on the basic block head; this amounts to the very same insertion place > as inserting after the basic block head. Also, we will continue to set no > location, and use the same bb, because both add_insn_before and > add_insn_after (in contradiction to its block comment) will infer the basic > block from the insn given (in the case for add_insn_before, I assume > that the basic block doesn't start with a BARRIER - that would be invalid - > and that the insn it starts with has a valid BLOCK_FOR_INSN setting the > same way the basic block head has. This looks reasonable, but I think that we need more commentary because it's not straightforward to understand, so I would: 1. explicitly state that we enforce an order on the entities in addition to the order on priority, both in the code (for example create a 4th paragraph in the comment at the top of the file, before "More details ...") and in the doc as you already did, but "ordering" the two orders for the sake of clarity: first the order on priority then, for the same priority, the order to the entities. 2. add a line in the head comment of new_seginfo saying that INSN may not be a NOTE_BASIC_BLOCK, unless BB is empty. 3. add a comment above the trick in optimize_mode_switching saying that it is both required to implement the FIFO insertion and valid because we know that the basic block was initially empty. It's not clear to me whether this is a regression or not, so you'll also need to run it by the RMs. In the meantime I have installed the attached patchlet. 2014-03-28 Eric Botcazou * mode-switching.c: Make small adjustments to the top comment. Index: mode-switching.c =================================================================== --- mode-switching.c (revision 208879) +++ mode-switching.c (working copy) @@ -45,20 +45,20 @@ along with GCC; see the file COPYING3. and finding all the insns which require a specific mode. Each insn gets a unique struct seginfo element. These structures are inserted into a list for each basic block. For each entity, there is an array of bb_info over - the flow graph basic blocks (local var 'bb_info'), and contains a list + the flow graph basic blocks (local var 'bb_info'), which contains a list of all insns within that basic block, in the order they are encountered. For each entity, any basic block WITHOUT any insns requiring a specific - mode are given a single entry, without a mode. (Each basic block - in the flow graph must have at least one entry in the segment table.) + mode are given a single entry without a mode (each basic block in the + flow graph must have at least one entry in the segment table). The LCM algorithm is then run over the flow graph to determine where to - place the sets to the highest-priority value in respect of first the first + place the sets to the highest-priority mode with respect to the first insn in any one block. Any adjustments required to the transparency vectors are made, then the next iteration starts for the next-lower priority mode, till for each entity all modes are exhausted. - More details are located in the code for optimize_mode_switching(). */ + More details can be found in the code of optimize_mode_switching. */ /* This structure contains the information for each insn which requires either single or double mode to be set.