From patchwork Thu Oct 25 09:45:48 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 194089 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 9F09E2C00AE for ; Thu, 25 Oct 2012 20:46:02 +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=1351763162; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:From:To:Mail-Followup-To:Cc:Subject:Date: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=vngZtiTTkVTxu9R8udxwd/eE8xA=; b=az63f9+Y4fUlCIK cUU5mFhqsFYElqVhXX1yhfK6UgEIqBnbcnGJU5MvyrLRBo43t9rUPkP357a0zbF2 x+xbuCrdtSEtJ0x4jEQmw7Hn2TjyB4hn+jTzheiqzdP4WUpu8mgPza4uO0rX4Toq 3X2XD/fm5/tpj6D8UA+3K1T6iCgA= 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:Date: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=aFMhk0AXntkZuN4yrWgb4TqiXeUhi3YeWNhhMgHQoV3SX610CaHPS/w13R5ROM DH74RufXxo+xzefPnSyVq19z+y2UztUaWLSrZBk8oklLC+fwJyVRMf9Zo27/viMs vUcz8ngvu5eikCD1GR+yMg6sy7nZ18J7z1PF0WSKoijik=; Received: (qmail 6373 invoked by alias); 25 Oct 2012 09:45:58 -0000 Received: (qmail 6364 invoked by uid 22791); 25 Oct 2012 09:45:57 -0000 X-SWARE-Spam-Status: No, hits=-3.8 required=5.0 tests=AWL, BAYES_00, 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-ea0-f175.google.com (HELO mail-ea0-f175.google.com) (209.85.215.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 25 Oct 2012 09:45:50 +0000 Received: by mail-ea0-f175.google.com with SMTP id c1so501150eaa.20 for ; Thu, 25 Oct 2012 02:45:49 -0700 (PDT) Received: by 10.14.214.2 with SMTP id b2mr25644442eep.32.1351158349331; Thu, 25 Oct 2012 02:45:49 -0700 (PDT) Received: from localhost ([2.26.192.222]) by mx.google.com with ESMTPS id c6sm29759160eep.17.2012.10.25.02.45.46 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 25 Oct 2012 02:45:48 -0700 (PDT) From: Richard Sandiford To: gcc-patches@gcc.gnu.org Mail-Followup-To: gcc-patches@gcc.gnu.org, vmakarov@redhat.com, rdsandiford@googlemail.com Cc: vmakarov@redhat.com Subject: RFA: displacement handling in equiv_address_substitution Date: Thu, 25 Oct 2012 10:45:48 +0100 Message-ID: <87a9vanaur.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 Hi Vlad, As discussed in the reviews, one of the things that worried me was the combination of: 1) the displacement fixup code in process_address assumes that the address is exactly equal to BASE_LOC + INDEX_LOC + DISP (with null values being equivalent to 0). 2) extract_address_regs allows (and has to allow) much more general forms than that. 3) equiv_address_substitution folds base and index displacements without checking the shape of the address. So the code from (1) could end up fixing a displacement created by (3) even if the address isn't a simple BASE_LOC + INDEX_LOC + DISP. There's a similar problem with the relationship between INDEX_LOC and INDEX_REG_LOC: we assume the scale is 1 unless INDEX is a MULT of the index register. This patch adds two utility functions, one to test whether the address is simple enough for the fixup code to handle, and one to see whether the index scale is known. The latter handles ASHIFT as well as MULT, since ASHIFT can be used in out-of-MEM address operands. The condition: /* If the address already has a displacement, we can simply add the new displacement to it. */ if (ad->disp_loc) return true; might leave a bit of a hole, but the asserts you added to extract_address_regs should mean that disp_loc really is a displacement (thanks for those btw). In principle, we could apply a base or index displacement even in cases that process_address can't fix up, test whether the result is valid, and revert to the normal behaviour of reloading the full base or index value if not. That's not going to be useful for x86 (or for MIPS :-)) and I'm not planning to do it, but the valid_address_p patch I just sent ought to help. Tested on x86_64-linux-gnu. Also tested by making sure that there were no changes in assembly output for a set of gcc .ii files (i.e. these extra checks didn't affect the code on x86_64 at least). OK to install? Richard gcc/ * lra-constraints.c (get_index_scale, can_add_disp_p): New functions. (equiv_address_substitution): Use them. Index: gcc/lra-constraints.c =================================================================== --- gcc/lra-constraints.c 2012-10-25 09:50:13.005284668 +0100 +++ gcc/lra-constraints.c 2012-10-25 09:55:17.376283922 +0100 @@ -756,6 +756,28 @@ extract_address_regs (enum machine_mode ad->index_loc = ad->index_reg_loc; } +/* Return the scale applied to *AD->INDEX_REG_LOC, or 0 if the index is + more complicated than that. */ +static HOST_WIDE_INT +get_index_scale (struct address *ad) +{ + rtx index = *ad->index_loc; + if (GET_CODE (index) == MULT + && CONST_INT_P (XEXP (index, 1)) + && ad->index_reg_loc == &XEXP (index, 0)) + return INTVAL (XEXP (index, 1)); + + if (GET_CODE (index) == ASHIFT + && CONST_INT_P (XEXP (index, 1)) + && ad->index_reg_loc == &XEXP (index, 0)) + return (HOST_WIDE_INT) 1 << INTVAL (XEXP (index, 1)); + + if (ad->index_reg_loc == ad->index_loc) + return 1; + + return 0; +} + /* The page contains major code to choose the current insn alternative @@ -2430,6 +2452,40 @@ base_plus_disp_to_reg (enum machine_mode return new_reg; } +/* Return true if we can add a displacement to address ADDR_LOC, + which is described by AD, even if that makes the address invalid. + The fix-up code requires any new address to be the sum of the base, + index and displacement fields of an AD-like structure. */ +static bool +can_add_disp_p (struct address *ad, rtx *addr_loc) +{ + /* Automodified addresses have a fixed form. */ + if (ad->base_modify_p) + return false; + + /* If the address already has a displacement, we can simply add the + new displacement to it. */ + if (ad->disp_loc) + return true; + + /* If the address is entirely a base or index, we can try adding + a constant to it. */ + if (addr_loc == ad->base_reg_loc || addr_loc == ad->index_loc) + return true; + + /* Likewise if the address is entirely a sum of the base and index. */ + if (GET_CODE (*addr_loc) == PLUS) + { + rtx *op0 = &XEXP (*addr_loc, 0); + rtx *op1 = &XEXP (*addr_loc, 1); + if (op0 == ad->base_reg_loc && op1 == ad->index_loc) + return true; + if (op1 == ad->base_reg_loc && op0 == ad->index_loc) + return true; + } + return false; +} + /* Make substitution in address AD in space AS with location ADDR_LOC. Update AD and ADDR_LOC if it is necessary. Return true if a substitution was made. */ @@ -2475,7 +2531,8 @@ equiv_address_substitution (struct addre } else if (GET_CODE (new_base_reg) == PLUS && REG_P (XEXP (new_base_reg, 0)) - && CONST_INT_P (XEXP (new_base_reg, 1))) + && CONST_INT_P (XEXP (new_base_reg, 1)) + && can_add_disp_p (ad, addr_loc)) { disp += INTVAL (XEXP (new_base_reg, 1)); *ad->base_reg_loc = XEXP (new_base_reg, 0); @@ -2484,12 +2541,6 @@ equiv_address_substitution (struct addre if (ad->base_reg_loc2 != NULL) *ad->base_reg_loc2 = *ad->base_reg_loc; } - scale = 1; - if (ad->index_loc != NULL && GET_CODE (*ad->index_loc) == MULT) - { - lra_assert (CONST_INT_P (XEXP (*ad->index_loc, 1))); - scale = INTVAL (XEXP (*ad->index_loc, 1)); - } if (index_reg != new_index_reg) { if (REG_P (new_index_reg)) @@ -2499,7 +2550,9 @@ equiv_address_substitution (struct addre } else if (GET_CODE (new_index_reg) == PLUS && REG_P (XEXP (new_index_reg, 0)) - && CONST_INT_P (XEXP (new_index_reg, 1))) + && CONST_INT_P (XEXP (new_index_reg, 1)) + && can_add_disp_p (ad, addr_loc) + && (scale = get_index_scale (ad))) { disp += INTVAL (XEXP (new_index_reg, 1)) * scale; *ad->index_reg_loc = XEXP (new_index_reg, 0);