On 21/02/17 16:54, charles.bay...@linaro.org wrote:
> From: Charles Baylis <charles.bay...@linaro.org>
>
> 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:
>
> <date> Charles Baylis <charles.bay...@linaro.org>
>
> * 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).
> + /* TODO: Add table-driven costs for addressing modes. */
You need to sort out the comment. What's missing here?
> + }
> +
> + /* 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.
> + *cost += COSTS_N_INSNS((GET_MODE_SIZE (mode) + bus_width - 1) /
> bus_width);
Use CEIL (from system.h)
> + *cost += extra_cost->ldst.load;
> + } else {
> + *cost += COSTS_N_INSNS (1);
> + }
> +
> + return true;
> +}
> +/* Convert fron bytes to ints. */
> +#define ARM_NUM_INTS(X) (((X) + UNITS_PER_WORD - 1) / UNITS_PER_WORD)
> +
> +
> /* RTX costs. Make an estimate of the cost of executing the operation
> X, which is contained with an operation with code OUTER_CODE.
> SPEED_P indicates whether the cost desired is the performance cost,
> @@ -9152,30 +9193,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:
> {
>
