From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 40414 invoked by alias); 9 Jun 2017 14:13:37 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 40385 invoked by uid 89); 9 Jun 2017 14:13:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=laid, Capital X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 09 Jun 2017 14:13:33 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8D3E280D; Fri, 9 Jun 2017 07:13:36 -0700 (PDT) Received: from e105689-lin.cambridge.arm.com (e105689-lin.cambridge.arm.com [10.2.207.32]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 759163F557; Fri, 9 Jun 2017 07:13:35 -0700 (PDT) Subject: Re: [PATCH 2/2] [ARM] Add table of costs for AAarch32 addressing modes. To: charles.baylis@linaro.org, Ramana.Radhakrishnan@arm.com, kyrylo.tkachov@arm.com Cc: rearnsha@arm.com, gcc-patches@gcc.gnu.org References: <1487696064-3233-1-git-send-email-charles.baylis@linaro.org> <1487696064-3233-3-git-send-email-charles.baylis@linaro.org> From: "Richard Earnshaw (lists)" Message-ID: <3df367f9-f987-f84a-5cc9-bc4c7d4c1ecc@arm.com> Date: Fri, 09 Jun 2017 14:13:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <1487696064-3233-3-git-send-email-charles.baylis@linaro.org> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-SW-Source: 2017-06/txt/msg00655.txt.bz2 On 21/02/17 16:54, charles.baylis@linaro.org wrote: > From: Charles Baylis > > This patch adds support for modelling the varying costs of > different addressing modes. The generic cost table treats > all addressing modes as having equal cost. The cost table > for Cortex-A57 is derived from http://infocenter.arm.com/help/topic/com.arm.doc.uan0015b/Cortex_A57_Software_Optimization_Guide_external.pdf > and treats addressing modes with write-back as having a > cost equal to one additional instruction. > > gcc/ChangeLog: > > Charles Baylis > > * config/arm/aarch-common-protos.h (enum arm_addr_mode_op): New. > (struct addr_mode_cost_table): New. > (struct cpu_cost_table): Add pointer to an addr_mode_cost_table. > * config/arm/aarch-cost-tables.h: (generic_addr_mode_costs): New. > (generic_extra_costs) Initialise aarch32_addr_mode. > (cortexa53_extra_costs) Likewise. > (addr_mode_costs_cortexa57) New. > (cortexa57_extra_costs) Initialise aarch32_addr_mode. > (exynosm1_extra_costs) Likewise. > (xgene1_extra_costs) Likewise. > (qdf24xx_extra_costs) Likewise. > * config/arm/arm.c (cortexa9_extra_costs) Initialise aarch32_addr_mode. > (cortexa9_extra_costs) Likewise. > (cortexa8_extra_costs) Likewise. > (cortexa5_extra_costs) Likewise. > (cortexa7_extra_costs) Likewise. > (cortexa12_extra_costs) Likewise. > (cortexv7m_extra_costs) Likewise. > (arm_mem_costs): Use table lookup to calculate cost of addressing > mode. > > Change-Id: If71bd7c4f4bb876c5ed82dc28791130efb8bf89e > --- > gcc/config/arm/aarch-common-protos.h | 16 +++++++++++ > gcc/config/arm/aarch-cost-tables.h | 54 ++++++++++++++++++++++++++++++---- > gcc/config/arm/arm.c | 56 ++++++++++++++++++++++++++++++------ > 3 files changed, 113 insertions(+), 13 deletions(-) > > diff --git a/gcc/config/arm/aarch-common-protos.h b/gcc/config/arm/aarch-common-protos.h > index 7c2bb4c..f6fcc94 100644 > --- a/gcc/config/arm/aarch-common-protos.h > +++ b/gcc/config/arm/aarch-common-protos.h > @@ -130,6 +130,21 @@ struct vector_cost_table > const int alu; > }; > > +enum arm_addr_mode_op > +{ > + AMO_DEFAULT, > + AMO_NO_WB, > + AMO_WB, > + AMO_MAX /* for array size */ Comment style: Capital letter at start, full stop and two spaces at the end. The enum and structure below should have some comments explaining what they are for. > +}; > + > +struct addr_mode_cost_table > +{ > + const int integer[AMO_MAX]; > + const int fp[AMO_MAX]; > + const int vector[AMO_MAX]; > +}; > + > struct cpu_cost_table > { > const struct alu_cost_table alu; > @@ -137,6 +152,7 @@ struct cpu_cost_table > const struct mem_cost_table ldst; > const struct fp_cost_table fp[2]; /* SFmode and DFmode. */ > const struct vector_cost_table vect; > + const struct addr_mode_cost_table *aarch32_addr_mode; > }; > > > diff --git a/gcc/config/arm/aarch-cost-tables.h b/gcc/config/arm/aarch-cost-tables.h > index 68f84b0..e3d257f 100644 > --- a/gcc/config/arm/aarch-cost-tables.h > +++ b/gcc/config/arm/aarch-cost-tables.h > @@ -22,6 +22,16 @@ > #ifndef GCC_AARCH_COST_TABLES_H > #define GCC_AARCH_COST_TABLES_H > > +const struct addr_mode_cost_table generic_addr_mode_costs = > +{ > + /* int */ > + { 0, 0, 0 }, Elements need to be commented, otherwise minor changes to the contents of the arrays is hard to update and maintain. > + /* float */ > + { 0, 0, 0 }, > + /* vector */ > + { 0, 0, 0 } > +}; > + > const struct cpu_cost_table generic_extra_costs = > { > /* ALU */ > @@ -122,7 +132,9 @@ const struct cpu_cost_table generic_extra_costs = > /* Vector */ > { > COSTS_N_INSNS (1) /* alu. */ > - } > + }, > + /* Addressing mode */ Comment style. > + &generic_addr_mode_costs > }; > > const struct cpu_cost_table cortexa53_extra_costs = > @@ -225,6 +237,30 @@ const struct cpu_cost_table cortexa53_extra_costs = > /* Vector */ > { > COSTS_N_INSNS (1) /* alu. */ > + }, > + /* Addressing mode */ > + &generic_addr_mode_costs > +}; > + > +const struct addr_mode_cost_table addr_mode_costs_cortexa57 = > +{ > + /* int */ > + { > + 0, > + 0, > + COSTS_N_INSNS (1), > + }, > + /* float */ > + { > + 0, > + 0, > + COSTS_N_INSNS (1), > + }, > + /* vector */ > + { > + 0, > + 0, > + COSTS_N_INSNS (1), > } > }; > > @@ -328,7 +364,9 @@ const struct cpu_cost_table cortexa57_extra_costs = > /* Vector */ > { > COSTS_N_INSNS (1) /* alu. */ > - } > + }, > + /* Addressing mode */ > + &addr_mode_costs_cortexa57 > }; > > const struct cpu_cost_table exynosm1_extra_costs = > @@ -431,7 +469,9 @@ const struct cpu_cost_table exynosm1_extra_costs = > /* Vector */ > { > COSTS_N_INSNS (0) /* alu. */ > - } > + }, > + /* Addressing mode */ > + &generic_addr_mode_costs > }; > > const struct cpu_cost_table xgene1_extra_costs = > @@ -534,7 +574,9 @@ const struct cpu_cost_table xgene1_extra_costs = > /* Vector */ > { > COSTS_N_INSNS (2) /* alu. */ > - } > + }, > + /* Addressing mode */ > + &generic_addr_mode_costs > }; > > const struct cpu_cost_table qdf24xx_extra_costs = > @@ -637,7 +679,9 @@ const struct cpu_cost_table qdf24xx_extra_costs = > /* Vector */ > { > COSTS_N_INSNS (1) /* alu. */ > - } > + }, > + /* Addressing mode */ > + &generic_addr_mode_costs > }; > > #endif /* GCC_AARCH_COST_TABLES_H */ > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index 7f002f1..fe4cd73 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -1099,7 +1099,9 @@ const struct cpu_cost_table cortexa9_extra_costs = > /* Vector */ > { > COSTS_N_INSNS (1) /* alu. */ > - } > + }, > + /* Addressing mode */ > + &generic_addr_mode_costs > }; > > const struct cpu_cost_table cortexa8_extra_costs = > @@ -1202,7 +1204,9 @@ const struct cpu_cost_table cortexa8_extra_costs = > /* Vector */ > { > COSTS_N_INSNS (1) /* alu. */ > - } > + }, > + /* Addressing mode */ > + &generic_addr_mode_costs > }; > > const struct cpu_cost_table cortexa5_extra_costs = > @@ -1306,7 +1310,9 @@ const struct cpu_cost_table cortexa5_extra_costs = > /* Vector */ > { > COSTS_N_INSNS (1) /* alu. */ > - } > + }, > + /* Addressing mode */ > + &generic_addr_mode_costs > }; > > > @@ -1411,7 +1417,9 @@ const struct cpu_cost_table cortexa7_extra_costs = > /* Vector */ > { > COSTS_N_INSNS (1) /* alu. */ > - } > + }, > + /* Addressing mode */ > + &generic_addr_mode_costs > }; > > const struct cpu_cost_table cortexa12_extra_costs = > @@ -1514,7 +1522,9 @@ const struct cpu_cost_table cortexa12_extra_costs = > /* Vector */ > { > COSTS_N_INSNS (1) /* alu. */ > - } > + }, > + /* Addressing mode */ > + &generic_addr_mode_costs > }; > > const struct cpu_cost_table cortexa15_extra_costs = > @@ -1617,7 +1627,9 @@ const struct cpu_cost_table cortexa15_extra_costs = > /* Vector */ > { > COSTS_N_INSNS (1) /* alu. */ > - } > + }, > + /* Addressing mode */ > + &generic_addr_mode_costs > }; > > const struct cpu_cost_table v7m_extra_costs = > @@ -1720,7 +1732,9 @@ const struct cpu_cost_table v7m_extra_costs = > /* Vector */ > { > COSTS_N_INSNS (1) /* alu. */ > - } > + }, > + /* Addressing mode */ > + &generic_addr_mode_costs > }; > > const struct tune_params arm_slowmul_tune = > @@ -9093,7 +9107,33 @@ arm_mem_costs (rtx x, const struct cpu_cost_table *extra_cost, > /* Calculate cost of the addressing mode. */ > if (speed_p) > { > - /* TODO: Add table-driven costs for addressing modes. */ > + arm_addr_mode_op op_type; > + switch (GET_CODE(XEXP (x, 0))) > + { > + case REG: > + default: > + op_type = AMO_DEFAULT; Why would REG and default share the same cost? Presumably default is too complex to recognize, but then it's unlikely to be cheap. > + break; > + case PLUS: > + case MINUS: > + op_type = AMO_NO_WB; GCC doesn't support MINUS in addresses, even though the architecture could in theory handle this. Also, I think you need a different cost for scaled offset addressing, and possibly even for different scaling factors and types of scaling. > + break; > + case PRE_INC: > + case PRE_DEC: > + case POST_INC: > + case POST_DEC: > + case PRE_MODIFY: > + case POST_MODIFY: > + op_type = AMO_WB; Pre and post might also need separate entries (plus further entries for scaling). A post operation might happen in parallel with the data fetch, while a pre operation must happen before the address can be sent to the load/store pipe. > + break; > + } blank line between the switch block and the subsequent statements. Also, the following needs to laid out in GNU style. > + if (VECTOR_MODE_P (mode)) { > + *cost += extra_cost->aarch32_addr_mode->vector[op_type]; > + } else if (FLOAT_MODE_P (mode)) { > + *cost += extra_cost->aarch32_addr_mode->fp[op_type]; > + } else { > + *cost += extra_cost->aarch32_addr_mode->integer[op_type]; > + } > } > > /* cost of memory access */ >