From patchwork Fri Aug 25 17:43:26 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Charles Baylis X-Patchwork-Id: 805986 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-460959-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="mVfKgFi6"; dkim-atps=neutral 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 3xf7nB1xBxz9sPk for ; Sat, 26 Aug 2017 03:43:41 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; q=dns; s=default; b=wBgKMERJm5OlSm7 qfRDtVyUQmJw7iN5d6j3nOFogpJEjT3/oRIvAG3M+BhDWP7+VEXT00Cr8aS9xz4Z W+pWpv+uRPBDGnG7xPR9qUYdfIG/GeNpJWl5SZShWPGeOFlDsuusd9pwirevLrWK /2U3a4KElWud/T7ChnP9G8N3be3o= 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 :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; s=default; bh=h+B5sVjTN/N7MaEF19F2O v/uLYw=; b=mVfKgFi6Af0t+cdglFHX4oP5pr7HZBKb0ZdzdSbuQ69bax+RFpZaw J9mGOHQtDQzlCIxTrAYZLBkHGcWlO+N3PN4J/d2UvVvprMSDVmMrWdDkceg0rfFl zdCJ0vl4+fvMsalM3RpF3aq/9mGNSiYthNYd28dTokFuijVb3PkVnY= Received: (qmail 53601 invoked by alias); 25 Aug 2017 17:43:32 -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 52136 invoked by uid 89); 25 Aug 2017 17:43:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=charles, Charles, HX-Envelope-From:sk:charles, divided X-HELO: mail-yw0-f176.google.com Received: from mail-yw0-f176.google.com (HELO mail-yw0-f176.google.com) (209.85.161.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 25 Aug 2017 17:43:28 +0000 Received: by mail-yw0-f176.google.com with SMTP id h127so2723596ywf.3 for ; Fri, 25 Aug 2017 10:43:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=XayOWrKptbQsayAg+FCUvo4CaW5Hz19TRIR/ZeCl5cU=; b=WIfdrkLlbVJtERMuwX4M3tEyAFwqdz572yrp+gLfz8uJXzVjSCLm2YgJ2TpciJZqsz JvJfG55EuP1+q0axf743qo/t0myg/q4qpobNmZFBqSAqfjTXTzN84MqlS7NVtUq321JB sLZzVq6Jw+q7/LtbqYC5nbQTCTTwPuBwDZSZ5ioBlQw96bizQq0kImi3vsPccymt7KgE CxlnOcZMV/oqOFd0+gdYr7IBm40gtZD05Ci5T36nF2Za8b+Z9+tVgXqHiqUMFv6qDI1e N+iU6L2ulp9x6VPXq9Rw/tUl2JQLqfHfgJjO5rRMH94i4t/oHx88PXVukzVVTElGpTNc 3twQ== X-Gm-Message-State: AHYfb5gzBHsqPpD4mfkIm3cV4O06dLPUCJix3sCoQYCPfDVD/rszeEf4 WaSu6oULh6eo1dNvfKxlS+Sa1pi81UXP X-Received: by 10.129.182.78 with SMTP id h14mr2245693ywk.154.1503683006898; Fri, 25 Aug 2017 10:43:26 -0700 (PDT) MIME-Version: 1.0 Received: by 10.129.46.201 with HTTP; Fri, 25 Aug 2017 10:43:26 -0700 (PDT) In-Reply-To: <16a7e0b2-a1d4-424a-5dde-85d32bedcb0a@arm.com> References: <1487696064-3233-1-git-send-email-charles.baylis@linaro.org> <1487696064-3233-2-git-send-email-charles.baylis@linaro.org> <16a7e0b2-a1d4-424a-5dde-85d32bedcb0a@arm.com> From: Charles Baylis Date: Fri, 25 Aug 2017 18:43:26 +0100 Message-ID: Subject: Re: [PATCH 1/2] [ARM] Refactor costs calculation for MEM. To: "Richard Earnshaw (lists)" Cc: Ramana Radhakrishnan , Kyrylo Tkachov , Richard Earnshaw , GCC Patches X-IsSubscribed: yes On 9 June 2017 at 14:59, Richard Earnshaw (lists) wrote: > On 21/02/17 16:54, charles.baylis@linaro.org wrote: >> From: Charles Baylis >> >> This patch moves the calculation of costs for MEM into a >> separate function, and reforms the calculation into two >> parts. Firstly any additional cost of the addressing mode >> is calculated, and then the cost of the memory access itself >> is added. >> >> In this patch, the calculation of the cost of the addressing >> mode is left as a placeholder, to be added in a subsequent >> patch. >> >> gcc/ChangeLog: >> >> Charles Baylis >> >> * config/arm/arm.c (arm_mem_costs): New function. >> (arm_rtx_costs_internal): Use arm_mem_costs. > > I like the idea of this patch, but it needs further work... > > Comments inline. > > R. > >> >> Change-Id: I99e93406ea39ee31f71c7bf428ad3e127b7a618e >> --- >> gcc/config/arm/arm.c | 66 +++++++++++++++++++++++++++++++++------------------- >> 1 file changed, 42 insertions(+), 24 deletions(-) >> >> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c >> index 6cae178..7f002f1 100644 >> --- a/gcc/config/arm/arm.c >> +++ b/gcc/config/arm/arm.c >> @@ -9072,6 +9072,47 @@ arm_unspec_cost (rtx x, enum rtx_code /* outer_code */, bool speed_p, int *cost) >> } \ >> while (0); >> >> +/* Helper function for arm_rtx_costs_internal. Calculates the cost of a MEM, >> + considering the costs of the addressing mode and memory access >> + separately. */ >> +static bool >> +arm_mem_costs (rtx x, const struct cpu_cost_table *extra_cost, >> + int *cost, bool speed_p) >> +{ >> + machine_mode mode = GET_MODE (x); >> + if (flag_pic >> + && GET_CODE (XEXP (x, 0)) == PLUS >> + && will_be_in_index_register (XEXP (XEXP (x, 0), 1))) >> + /* This will be split into two instructions. Add the cost of the >> + additional instruction here. The cost of the memory access is computed >> + below. See arm.md:calculate_pic_address. */ >> + *cost = COSTS_N_INSNS (1); >> + else >> + *cost = 0; >> + >> + /* Calculate cost of the addressing mode. */ >> + if (speed_p) >> + { > > This patch needs to be reformatted in the GNU style (indentation of > braces, braces and else clauses on separate lines etc). Done. >> + /* TODO: Add table-driven costs for addressing modes. */ > > You need to sort out the comment. What's missing here? What's missing is patch 2... I've updated the comment for clarity. >> + } >> + >> + /* cost of memory access */ >> + if (speed_p) >> + { >> + /* data transfer is transfer size divided by bus width. */ >> + int bus_width = arm_arch7 ? 8 : 4; > > Basing bus width on the architecture is a bit too simplistic. Instead > this should be a parameter that comes from the CPU cost tables, based on > the current tune target. This was actually Ramana's suggestion, so I've left it as-is in this patch. If necessary, I think it's better to move this to a table in a separate patch, as I'll need to guess the correct bus width for a number of CPUs and will probably get some wrong. >> + *cost += COSTS_N_INSNS((GET_MODE_SIZE (mode) + bus_width - 1) / bus_width); > > Use CEIL (from system.h) Done. Updated patch attached. From 18629835ba12fdfa693e2f9492a5fc23d95ef165 Mon Sep 17 00:00:00 2001 From: Charles Baylis Date: Wed, 8 Feb 2017 16:52:10 +0000 Subject: [PATCH 1/3] [ARM] Refactor costs calculation for MEM. This patch moves the calculation of costs for MEM into a separate function, and reforms the calculation into two parts. Firstly any additional cost of the addressing mode is calculated, and then the cost of the memory access itself is added. In this patch, the calculation of the cost of the addressing mode is left as a placeholder, to be added in a subsequent patch. gcc/ChangeLog: Charles Baylis * config/arm/arm.c (arm_mem_costs): New function. (arm_rtx_costs_internal): Use arm_mem_costs. Change-Id: I99e93406ea39ee31f71c7bf428ad3e127b7a618e --- gcc/config/arm/arm.c | 67 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 25 deletions(-) diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index fa3e2fa..13cd421 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -9198,8 +9198,48 @@ arm_unspec_cost (rtx x, enum rtx_code /* outer_code */, bool speed_p, int *cost) } \ while (0); +/* Helper function for arm_rtx_costs_internal. Calculates the cost of a MEM, + considering the costs of the addressing mode and memory access + separately. */ +static bool +arm_mem_costs (rtx x, const struct cpu_cost_table *extra_cost, + int *cost, bool speed_p) +{ + machine_mode mode = GET_MODE (x); + if (flag_pic + && GET_CODE (XEXP (x, 0)) == PLUS + && will_be_in_index_register (XEXP (XEXP (x, 0), 1))) + /* This will be split into two instructions. Add the cost of the + additional instruction here. The cost of the memory access is computed + below. See arm.md:calculate_pic_address. */ + *cost = COSTS_N_INSNS (1); + else + *cost = 0; + + /* Calculate cost of the addressing mode. */ + if (speed_p) + { + /* TODO: Add table-driven costs for addressing modes. (See patch 2) */ + } + + /* Calculate cost of memory access. */ + if (speed_p) + { + /* data transfer is transfer size divided by bus width. */ + int bus_width = arm_arch7 ? 8 : 4; + *cost += CEIL (GET_MODE_SIZE (mode), bus_width); + *cost += extra_cost->ldst.load; + } + else + { + *cost += COSTS_N_INSNS (1); + } + + return true; +} + /* RTX costs. Make an estimate of the cost of executing the operation - X, which is contained with an operation with code OUTER_CODE. + X, which is contained within an operation with code OUTER_CODE. SPEED_P indicates whether the cost desired is the performance cost, or the size cost. The estimate is stored in COST and the return value is TRUE if the cost calculation is final, or FALSE if the @@ -9278,30 +9318,7 @@ arm_rtx_costs_internal (rtx x, enum rtx_code code, enum rtx_code outer_code, return false; case MEM: - /* A memory access costs 1 insn if the mode is small, or the address is - a single register, otherwise it costs one insn per word. */ - if (REG_P (XEXP (x, 0))) - *cost = COSTS_N_INSNS (1); - else if (flag_pic - && GET_CODE (XEXP (x, 0)) == PLUS - && will_be_in_index_register (XEXP (XEXP (x, 0), 1))) - /* This will be split into two instructions. - See arm.md:calculate_pic_address. */ - *cost = COSTS_N_INSNS (2); - else - *cost = COSTS_N_INSNS (ARM_NUM_REGS (mode)); - - /* For speed optimizations, add the costs of the address and - accessing memory. */ - if (speed_p) -#ifdef NOT_YET - *cost += (extra_cost->ldst.load - + arm_address_cost (XEXP (x, 0), mode, - ADDR_SPACE_GENERIC, speed_p)); -#else - *cost += extra_cost->ldst.load; -#endif - return true; + return arm_mem_costs (x, extra_cost, cost, speed_p); case PARALLEL: { -- 2.7.4