From patchwork Thu Oct 25 20:21:06 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 194306 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 3A6872C009A for ; Fri, 26 Oct 2012 07:21:23 +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=1351801284; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:From:To:Mail-Followup-To:Cc:Subject:References:Date: In-Reply-To:Message-ID:User-Agent:MIME-Version:Content-Type: Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:Sender:Delivered-To; bh=4zcm7VSvdKF2kAUGIFSU HvFJBIY=; b=eWbyo1wxslmaTg6O7f7qRnc2f39yQovha23BaSnk0+pqYD3yFvJR /ZDaBGBk+d1Bsv5syrG6jxXgnjzl08QSIDZjBZvT4w5J3NoNM4HZ3cpmp8ZgCTWP 49z2ssofyiQ/TqnJ8WXc/WAAERDL3d+f6d3pnU3qqL9nGEVcvuldbC8= 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:Received:From:To:Mail-Followup-To:Cc:Subject:References:Date:In-Reply-To:Message-ID:User-Agent:MIME-Version:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=P/VsIb0ks+39iyxUqC5EzmdJhfOLZx+xBAwYM9QNNcMQcRGRbVSqz8h6S9n5Lf iqSYgwSe+vpNJdsKHDdv7iXVWib/TQ3CvIxjtiBUfHrTHEG4IMaH6hEDsrPBXM/U BKGmOnQTGkSDzzpvwhyP3FAIpaBHadGkce17elCZYHcjM=; Received: (qmail 4139 invoked by alias); 25 Oct 2012 20:21:17 -0000 Received: (qmail 4118 invoked by uid 22791); 25 Oct 2012 20:21:12 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL, BAYES_50, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, DKIM_VALID, FREEMAIL_FROM, KHOP_RCVD_TRUST, NML_ADSP_CUSTOM_MED, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-we0-f175.google.com (HELO mail-we0-f175.google.com) (74.125.82.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 25 Oct 2012 20:21:07 +0000 Received: by mail-we0-f175.google.com with SMTP id t44so1135579wey.20 for ; Thu, 25 Oct 2012 13:21:05 -0700 (PDT) Received: by 10.216.213.28 with SMTP id z28mr13234621weo.47.1351196465526; Thu, 25 Oct 2012 13:21:05 -0700 (PDT) Received: from localhost ([2.26.192.222]) by mx.google.com with ESMTPS id dt9sm11597272wib.1.2012.10.25.13.21.02 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 25 Oct 2012 13:21:03 -0700 (PDT) From: Richard Sandiford To: Vladimir Makarov Mail-Followup-To: Vladimir Makarov , gcc-patches@gcc.gnu.org, rdsandiford@googlemail.com Cc: gcc-patches@gcc.gnu.org Subject: Re: RFA: Clarify requirements of process_address References: <87ip9ync4y.fsf@talisman.home> <508995ED.4030309@redhat.com> Date: Thu, 25 Oct 2012 21:21:06 +0100 In-Reply-To: <508995ED.4030309@redhat.com> (Vladimir Makarov's message of "Thu, 25 Oct 2012 15:41:33 -0400") Message-ID: <87a9val2vh.fsf@talisman.home> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 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 Vladimir Makarov writes: > On 10/25/2012 05:18 AM, Richard Sandiford wrote: >> Hi Vlad, >> >> When testing other patches, I was misled by: >> >> /* Addresses were legitimate before LRA. So if the address has >> two registers than it can have two of them. We should also >> not worry about scale for the same reason. */ >> >> which I took to mean that process_address only handles pre-LRA addresses. >> I see now that it actually has to handle addresses created within LRA too, >> some of which might never have been valid. That also explains why we have >> to handle invalid addresses that have no base or index, just a displacement. >> >> Here's an attempt to enumerate the cases. Does it look OK? >> Tested on x86_64-linux-gnu. >> > It is a good start. We definitely need to have a better understanding > GCC addresses therefore good comments are important. It is ok for me to > commit. Thanks for working on this. You make my life easier. > > But I guess it does not describe all cases, e.g. displacement was valid > but after updating offsets for eliminated regs (because stack slots were > allocated) it became invalid. Ah, yeah, thanks. How about this instead? Richard gcc/ * lra-constraints.c (process_address): Describe the kinds of address that we might see. Index: gcc/lra-constraints.c =================================================================== --- gcc/lra-constraints.c 2012-10-25 21:07:25.775369947 +0100 +++ gcc/lra-constraints.c 2012-10-25 21:18:30.203368322 +0100 @@ -2496,8 +2496,23 @@ equiv_address_substitution (struct addre return change_p; } -/* Major function to make reloads for address in operand NOP. Add to - reloads to the list *BEFORE and *AFTER. We might need to add +/* Major function to make reloads for an address in operand NOP. + The supported cases are: + + 1) an address that existed before LRA started, at which point it must + have been valid. These addresses are subject to elimination and + may have become invalid due to the elimination offset being out + of range. + + 2) an address created by forcing a constant to memory (force_const_to_mem). + The initial form of these addresses might not be valid, and it is this + function's job to make them valid. + + 3) a frame address formed from a register and a (possibly zero) + constant offset. As above, these addresses might not be valid + and this function must make them so. + + Add reloads to the lists *BEFORE and *AFTER. We might need to add reloads to *AFTER because of inc/dec, {pre, post} modify in the address. Return true for any RTL change. */ static bool @@ -2559,9 +2574,19 @@ process_address (int nop, rtx *before, r && process_addr_reg (ad.index_reg_loc, before, NULL, INDEX_REG_CLASS)) change_p = true; - /* The address was valid before LRA. We only change its form if the - address has a displacement, so if it has no displacement it must - still be valid. */ + /* There are three cases where the shape of *ADDR_LOC may now be invalid: + + 1) the original address was valid, but either elimination or + equiv_address_substitution applied a displacement that made + it invalid. + + 2) the address is an invalid symbolic address created by + force_const_to_mem. + + 3) the address is a frame address with an invalid offset. + + All these cases involve a displacement, so there is no point + revalidating when there is no displacement. */ if (ad.disp_loc == NULL) return change_p; @@ -2596,9 +2621,8 @@ process_address (int nop, rtx *before, r if (ok_p) return change_p; - /* Addresses were legitimate before LRA. So if the address has - two registers than it can have two of them. We should also - not worry about scale for the same reason. */ + /* Any index existed before LRA started, so we can assume that the + presence and shape of the index is valid. */ push_to_sequence (*before); if (ad.base_reg_loc == NULL) { @@ -2613,7 +2637,7 @@ process_address (int nop, rtx *before, r rtx insn; rtx last = get_last_insn (); - /* disp => lo_sum (new_base, disp) */ + /* disp => lo_sum (new_base, disp), case (2) above. */ insn = emit_insn (gen_rtx_SET (VOIDmode, new_reg, gen_rtx_HIGH (Pmode, copy_rtx (*ad.disp_loc)))); @@ -2635,14 +2659,15 @@ process_address (int nop, rtx *before, r #endif if (code < 0) { - /* disp => new_base */ + /* disp => new_base, case (2) above. */ lra_emit_move (new_reg, *ad.disp_loc); *ad.disp_loc = new_reg; } } else { - /* index * scale + disp => new base + index * scale */ + /* index * scale + disp => new base + index * scale, + case (1) above. */ enum reg_class cl = base_reg_class (mode, as, SCRATCH, SCRATCH); lra_assert (INDEX_REG_CLASS != NO_REGS); @@ -2656,7 +2681,7 @@ process_address (int nop, rtx *before, r } else if (ad.index_reg_loc == NULL) { - /* base + disp => new base */ + /* base + disp => new base, cases (1) and (3) above. */ /* Another option would be to reload the displacement into an index register. However, postreload has code to optimize address reloads that have the same base and different @@ -2667,7 +2692,8 @@ process_address (int nop, rtx *before, r } else { - /* base + scale * index + disp => new base + scale * index */ + /* base + scale * index + disp => new base + scale * index, + case (1) above. */ new_reg = base_plus_disp_to_reg (mode, as, &ad); *addr_loc = gen_rtx_PLUS (Pmode, new_reg, *ad.index_loc); }