From: "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>
To: charles.baylis@linaro.org, Ramana.Radhakrishnan@arm.com,
kyrylo.tkachov@arm.com
Cc: rearnsha@arm.com, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH 2/2] [ARM] Add table of costs for AAarch32 addressing modes.
Date: Fri, 09 Jun 2017 14:13:00 -0000 [thread overview]
Message-ID: <3df367f9-f987-f84a-5cc9-bc4c7d4c1ecc@arm.com> (raw)
In-Reply-To: <1487696064-3233-3-git-send-email-charles.baylis@linaro.org>
On 21/02/17 16:54, charles.baylis@linaro.org wrote:
> From: Charles Baylis <charles.baylis@linaro.org>
>
> 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:
>
> <date> Charles Baylis <charles.baylis@linaro.org>
>
> * 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 */
>
next prev parent reply other threads:[~2017-06-09 14:13 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-21 16:54 [PATCH 0/2] [ARM] PR61551 addressing mode costs charles.baylis
2017-02-21 16:54 ` [PATCH 1/2] [ARM] Refactor costs calculation for MEM charles.baylis
2017-02-23 7:46 ` Bernhard Reutner-Fischer
2017-06-09 13:59 ` Richard Earnshaw (lists)
2017-08-25 18:16 ` Charles Baylis
2017-02-21 16:58 ` [PATCH 2/2] [ARM] Add table of costs for AAarch32 addressing modes charles.baylis
2017-06-09 14:13 ` Richard Earnshaw (lists) [this message]
2017-08-25 19:05 ` Charles Baylis
2017-08-25 19:10 ` Andrew Pinski
2017-05-12 15:53 ` [PATCH 0/2] [ARM] PR61551 addressing mode costs Charles Baylis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3df367f9-f987-f84a-5cc9-bc4c7d4c1ecc@arm.com \
--to=richard.earnshaw@arm.com \
--cc=Ramana.Radhakrishnan@arm.com \
--cc=charles.baylis@linaro.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=kyrylo.tkachov@arm.com \
--cc=rearnsha@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).