From patchwork Thu Sep 26 16:23:50 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 20629 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-qc0-f199.google.com (mail-qc0-f199.google.com [209.85.216.199]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 462D523A4E for ; Thu, 26 Sep 2013 16:24:30 +0000 (UTC) Received: by mail-qc0-f199.google.com with SMTP id u18sf1201797qcx.2 for ; Thu, 26 Sep 2013 09:24:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:from:to:mail-followup-to:cc:subject :references:date:in-reply-to:message-id:user-agent:mime-version :x-original-sender:x-original-authentication-results:precedence :mailing-list:list-id:list-post:list-help:list-archive :list-unsubscribe:content-type; bh=g+g/MSRDBu6Bu6KOcmCvHF4KZm9FU8fjdeDs3Jg51Yo=; b=UC3w6/OlqA+CUsbMIWD+btGIfHlJWRO7Ntwh958/q4yuPpKBonrXCojxOOOh8Dfapx PQHDn/YZGiGO6RsP/sSRAFAOlrYgktrMdMUFfioc36XwXUb9U2DpMe4BcIRc9fQq8oOz Fp93rfa/BCQ/VeFETcVsnUN2rqsvuxB+nWUilYLQdTTF2B7gNrjRhesCyrre7XE4WznY y/ilP7miIwEMjgSX+BHqq4Kj70PnfvgY8nEfZl6I4+syUMR7tWiHa8YM96m074r+RDrk Bcy//4hrolAzDqpm21IHpf2deXc+C+hUuQ+nF43rxnG+ygPRed0ZGwlpEz4bLI8EeH9m PUfg== X-Gm-Message-State: ALoCoQlXHbU5HVlAwxL007h97zyoZOw77hflzk3OrEnUEzIz///V07Pma7fwBFNF224zTyEugzZ9 X-Received: by 10.58.243.232 with SMTP id xb8mr156220vec.3.1380212669772; Thu, 26 Sep 2013 09:24:29 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.29.129 with SMTP id k1ls1010923qeh.75.gmail; Thu, 26 Sep 2013 09:24:29 -0700 (PDT) X-Received: by 10.220.181.136 with SMTP id by8mr1400666vcb.11.1380212669608; Thu, 26 Sep 2013 09:24:29 -0700 (PDT) Received: from mail-vb0-f50.google.com (mail-vb0-f50.google.com [209.85.212.50]) by mx.google.com with ESMTPS id tq4si586383vdc.142.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 26 Sep 2013 09:24:29 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.212.50 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.212.50; Received: by mail-vb0-f50.google.com with SMTP id x14so1019685vbb.37 for ; Thu, 26 Sep 2013 09:23:59 -0700 (PDT) X-Received: by 10.58.118.130 with SMTP id km2mr1420604veb.0.1380212639452; Thu, 26 Sep 2013 09:23:59 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.220.174.196 with SMTP id u4csp376105vcz; Thu, 26 Sep 2013 09:23:58 -0700 (PDT) X-Received: by 10.14.225.199 with SMTP id z47mr2609543eep.24.1380212637810; Thu, 26 Sep 2013 09:23:57 -0700 (PDT) Received: from e06smtp12.uk.ibm.com (e06smtp12.uk.ibm.com. [195.75.94.108]) by mx.google.com with ESMTPS id l4si2135732eew.341.1969.12.31.16.00.00 (version=TLSv1 cipher=RC4-SHA bits=128/128); Thu, 26 Sep 2013 09:23:57 -0700 (PDT) Received-SPF: pass (google.com: domain of rsandifo@linux.vnet.ibm.com designates 195.75.94.108 as permitted sender) client-ip=195.75.94.108; Received: from /spool/local by e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 26 Sep 2013 17:23:56 +0100 Received: from d06dlp01.portsmouth.uk.ibm.com (9.149.20.13) by e06smtp12.uk.ibm.com (192.168.101.142) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Thu, 26 Sep 2013 17:23:54 +0100 Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id 003FE17D8057; Thu, 26 Sep 2013 17:24:09 +0100 (BST) Received: from d06av04.portsmouth.uk.ibm.com (d06av04.portsmouth.uk.ibm.com [9.149.37.216]) by b06cxnps4075.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r8QGNfbl63438898; Thu, 26 Sep 2013 16:23:41 GMT Received: from d06av04.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av04.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r8QGNqmW030174; Thu, 26 Sep 2013 10:23:53 -0600 Received: from sandifor-thinkpad.stglab.manchester.uk.ibm.com (sig-9-145-199-228.de.ibm.com [9.145.199.228]) by d06av04.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id r8QGNocQ030116 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Thu, 26 Sep 2013 10:23:51 -0600 From: Richard Sandiford To: Eric Botcazou Mail-Followup-To: Eric Botcazou , gcc-patches@gcc.gnu.org, Yvan Roux , Vladimir Makarov , Richard Earnshaw , Marcus Shawcroft , Ramana Radhakrishnan , Matthew Gretton-Dann , Patch Tracking , Richard Henderson , rsandifo@linux.vnet.ibm.com Cc: gcc-patches@gcc.gnu.org, Yvan Roux , Vladimir Makarov , Richard Earnshaw , Marcus Shawcroft , Ramana Radhakrishnan , Matthew Gretton-Dann , Patch Tracking , Richard Henderson Subject: Re: [PATCH, ARM, LRA] Prepare ARM build with LRA References: <1623242.BDHJTW32D4@polaris> <87k3i4mx8i.fsf@talisman.default> <2041437.Gsg4dxHbXF@polaris> Date: Thu, 26 Sep 2013 17:23:50 +0100 In-Reply-To: <2041437.Gsg4dxHbXF@polaris> (Eric Botcazou's message of "Wed, 25 Sep 2013 23:16:26 +0200") Message-ID: <8761tnlfft.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 X-TM-AS-MML: No X-Content-Scanned: Fidelis XPS MAILER x-cbid: 13092616-8372-0000-0000-0000074D3ABE X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: patch@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.212.50 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , Eric Botcazou writes: >> So in the set_* routines it isn't about whether the value is definitely >> a base or a definitely an index. It's just about drilling down through >> what we've already decided is a base or index to get the inner reg or mem, >> and knowing which XEXPs to look at. We could instead have used a >> for_each_rtx, or something like that, without any code checks. But I >> wanted to be precise about the types of address we allow, so that we can >> assert for things we don't understand. In other words, it was "designed" >> to require the kind of extension Yvan is adding here. > > Does this mean that the design is to require a parallel implementation in the > predicates and in the set routines, i.e. each time you add a new case to the > predicates, you need to add it (or do something) to the set routines as well? > If so, that's a little weird, but OK, feel free to revert the de-duplication > part, but add comments saying that the functions must be kept synchronized. They don't need to be kept synchronised as such. It's fine for the index to allow more than must_be_index_p. But if you're not keen on the current structure, does the following look better? Tested on x86_64-linux-gnu. Thanks, Richard gcc/ * rtlanal.c (must_be_base_p, must_be_index_p): Delete. (binary_scale_code_p, get_base_term, get_index_term): New functions. (set_address_segment, set_address_base, set_address_index) (set_address_disp): Accept the argument unconditionally. (baseness): Remove must_be_base_p and must_be_index_p checks. (decompose_normal_address): Classify as much as possible in the main loop. Index: gcc/rtlanal.c =================================================================== --- gcc/rtlanal.c 2013-09-25 22:42:16.870221821 +0100 +++ gcc/rtlanal.c 2013-09-26 09:11:41.273778750 +0100 @@ -5521,26 +5521,50 @@ strip_address_mutations (rtx *loc, enum } } -/* Return true if X must be a base rather than an index. */ +/* Return true if CODE applies some kind of scale. The scaled value is + is the first operand and the scale is the second. */ static bool -must_be_base_p (rtx x) +binary_scale_code_p (enum rtx_code code) { - return GET_CODE (x) == LO_SUM; + return (code == MULT + || code == ASHIFT + /* Needed by ARM targets. */ + || code == ASHIFTRT + || code == LSHIFTRT + || code == ROTATE + || code == ROTATERT); } -/* Return true if X must be an index rather than a base. */ +/* If *INNER can be interpreted as a base, return a pointer to the inner term + (see address_info). Return null otherwise. */ -static bool -must_be_index_p (rtx x) +static rtx * +get_base_term (rtx *inner) { - return (GET_CODE (x) == MULT - || GET_CODE (x) == ASHIFT - /* Needed by ARM targets. */ - || GET_CODE (x) == ASHIFTRT - || GET_CODE (x) == LSHIFTRT - || GET_CODE (x) == ROTATE - || GET_CODE (x) == ROTATERT); + if (GET_CODE (*inner) == LO_SUM) + inner = strip_address_mutations (&XEXP (*inner, 0)); + if (REG_P (*inner) + || MEM_P (*inner) + || GET_CODE (*inner) == SUBREG) + return inner; + return 0; +} + +/* If *INNER can be interpreted as an index, return a pointer to the inner term + (see address_info). Return null otherwise. */ + +static rtx * +get_index_term (rtx *inner) +{ + /* At present, only constant scales are allowed. */ + if (binary_scale_code_p (GET_CODE (*inner)) && CONSTANT_P (XEXP (*inner, 1))) + inner = strip_address_mutations (&XEXP (*inner, 0)); + if (REG_P (*inner) + || MEM_P (*inner) + || GET_CODE (*inner) == SUBREG) + return inner; + return 0; } /* Set the segment part of address INFO to LOC, given that INNER is the @@ -5549,8 +5573,6 @@ must_be_index_p (rtx x) static void set_address_segment (struct address_info *info, rtx *loc, rtx *inner) { - gcc_checking_assert (GET_CODE (*inner) == UNSPEC); - gcc_assert (!info->segment); info->segment = loc; info->segment_term = inner; @@ -5562,12 +5584,6 @@ set_address_segment (struct address_info static void set_address_base (struct address_info *info, rtx *loc, rtx *inner) { - if (must_be_base_p (*inner)) - inner = strip_address_mutations (&XEXP (*inner, 0)); - gcc_checking_assert (REG_P (*inner) - || MEM_P (*inner) - || GET_CODE (*inner) == SUBREG); - gcc_assert (!info->base); info->base = loc; info->base_term = inner; @@ -5579,12 +5595,6 @@ set_address_base (struct address_info *i static void set_address_index (struct address_info *info, rtx *loc, rtx *inner) { - if (must_be_index_p (*inner) && CONSTANT_P (XEXP (*inner, 1))) - inner = strip_address_mutations (&XEXP (*inner, 0)); - gcc_checking_assert (REG_P (*inner) - || MEM_P (*inner) - || GET_CODE (*inner) == SUBREG); - gcc_assert (!info->index); info->index = loc; info->index_term = inner; @@ -5596,8 +5606,6 @@ set_address_index (struct address_info * static void set_address_disp (struct address_info *info, rtx *loc, rtx *inner) { - gcc_checking_assert (CONSTANT_P (*inner)); - gcc_assert (!info->disp); info->disp = loc; info->disp_term = inner; @@ -5677,12 +5685,6 @@ extract_plus_operands (rtx *loc, rtx **p baseness (rtx x, enum machine_mode mode, addr_space_t as, enum rtx_code outer_code, enum rtx_code index_code) { - /* See whether we can be certain. */ - if (must_be_base_p (x)) - return 3; - if (must_be_index_p (x)) - return -3; - /* Believe *_POINTER unless the address shape requires otherwise. */ if (REG_P (x) && REG_POINTER (x)) return 2; @@ -5717,8 +5719,8 @@ decompose_normal_address (struct address if (n_ops > 1) info->base_outer_code = PLUS; - /* Separate the parts that contain a REG or MEM from those that don't. - Record the latter in INFO and leave the former in OPS. */ + /* Try to classify each sum operand now. Leave those that could be + either a base or an index in OPS. */ rtx *inner_ops[4]; size_t out = 0; for (size_t in = 0; in < n_ops; ++in) @@ -5731,18 +5733,31 @@ decompose_normal_address (struct address set_address_segment (info, loc, inner); else { - ops[out] = loc; - inner_ops[out] = inner; - ++out; + /* The only other possibilities are a base or an index. */ + rtx *base_term = get_base_term (inner); + rtx *index_term = get_index_term (inner); + gcc_assert (base_term || index_term); + if (!base_term) + set_address_index (info, loc, index_term); + else if (!index_term) + set_address_base (info, loc, base_term); + else + { + gcc_assert (base_term == index_term); + ops[out] = loc; + inner_ops[out] = base_term; + ++out; + } } } /* Classify the remaining OPS members as bases and indexes. */ if (out == 1) { - /* Assume that the remaining value is a base unless the shape - requires otherwise. */ - if (!must_be_index_p (*inner_ops[0])) + /* If we haven't seen a base or an index yet, assume that this is + the base. If we were confident that another term was the base + or index, treat the remaining operand as the other kind. */ + if (!info->base) set_address_base (info, ops[0], inner_ops[0]); else set_address_index (info, ops[0], inner_ops[0]);