[2/2,ARM] Add table of costs for AAarch32 addressing modes.

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(-)

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 */
>