Re: [PATCH] [aarch64] Add CPU support for Ampere Computing's eMAG.

[Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>]

Hi Christoph,

Thank you for the patch.
Can you please confirm how this has been tested?

On 19/11/18 17:11, Christoph Muellner wrote:
> *** gcc/ChangeLog ***
>
> 2018-xx-xx  Christoph Muellner <christoph.muellner@theobroma-system.com>
>
> 	* config/aarch64/aarch64-cores.def: Define emag
> 	* config/aarch64/aarch64-tune.md: Regenerated with emag
> 	* config/aarch64/aarch64.c: Defining tuning struct

Please include the name of the new struct like so:
    * config/aarch64/aarch64.c (emag_tunings): New struct.

Also, full stops at the end of all entries.

> 	* doc/invoke.texi: Document mtune value
> ---
>   gcc/config/aarch64/aarch64-cores.def |  1 +
>   gcc/config/aarch64/aarch64-tune.md   |  2 +-
>   gcc/config/aarch64/aarch64.c         | 25 +++++++++++++++++++++++++
>   gcc/doc/invoke.texi                  |  2 +-
>   4 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64-cores.def b/gcc/config/aarch64/aarch64-cores.def
> index 1f3ac56..6e6800e 100644
> --- a/gcc/config/aarch64/aarch64-cores.def
> +++ b/gcc/config/aarch64/aarch64-cores.def
> @@ -63,6 +63,7 @@ AARCH64_CORE("thunderxt83",   thunderxt83,   thunderx,  8A,  AARCH64_FL_FOR_ARCH
>   
>   /* APM ('P') cores. */
>   AARCH64_CORE("xgene1",      xgene1,    xgene1,    8A,  AARCH64_FL_FOR_ARCH8, xgene1, 0x50, 0x000, -1)
> +AARCH64_CORE("emag",        emag,      xgene1,    8A,  AARCH64_FL_FOR_ARCH8, emag, 0x50, 0x000, -1)
>   

I'd suggest you start a new comment "section" here, something like /* Ampere cores.  */
From this definition this looks identical to xgene1. In particular the IMP and PART fields.
Are they really the same? You can find the values in /proc/cpuinfo on a GNU/Linux system.

If so, I don't think the -mcpu=native support will be able to pick up emag properly.
Do you have access to a Linux system running on this processor? What does -mcpu=native -### get rewritten to?

Thanks,
Kyrill

>   /* Qualcomm ('Q') cores. */
>   AARCH64_CORE("falkor",      falkor,    falkor,    8A,  AARCH64_FL_FOR_ARCH8 | AARCH64_FL_CRC | AARCH64_FL_CRYPTO | AARCH64_FL_RDMA, qdf24xx,   0x51, 0xC00, -1)
> diff --git a/gcc/config/aarch64/aarch64-tune.md b/gcc/config/aarch64/aarch64-tune.md
> index fade1d4..408976a 100644
> --- a/gcc/config/aarch64/aarch64-tune.md
> +++ b/gcc/config/aarch64/aarch64-tune.md
> @@ -1,5 +1,5 @@
>   ;; -*- buffer-read-only: t -*-
>   ;; Generated automatically by gentune.sh from aarch64-cores.def
>   (define_attr "tune"
> -	"cortexa35,cortexa53,cortexa57,cortexa72,cortexa73,thunderx,thunderxt88p1,thunderxt88,thunderxt81,thunderxt83,xgene1,falkor,qdf24xx,exynosm1,phecda,thunderx2t99p1,vulcan,thunderx2t99,cortexa55,cortexa75,cortexa76,ares,tsv110,saphira,cortexa57cortexa53,cortexa72cortexa53,cortexa73cortexa35,cortexa73cortexa53,cortexa75cortexa55,cortexa76cortexa55"
> +	"cortexa35,cortexa53,cortexa57,cortexa72,cortexa73,thunderx,thunderxt88p1,thunderxt88,thunderxt81,thunderxt83,xgene1,emag,falkor,qdf24xx,exynosm1,phecda,thunderx2t99p1,vulcan,thunderx2t99,cortexa55,cortexa75,cortexa76,ares,tsv110,saphira,cortexa57cortexa53,cortexa72cortexa53,cortexa73cortexa35,cortexa73cortexa53,cortexa75cortexa55,cortexa76cortexa55"
>   	(const (symbol_ref "((enum attr_tune) aarch64_tune)")))
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index f7f88a9..995aafe 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -957,6 +957,31 @@ static const struct tune_params xgene1_tunings =
>     &xgene1_prefetch_tune
>   };
>   
> +static const struct tune_params emag_tunings =
> +{
> +  &xgene1_extra_costs,
> +  &xgene1_addrcost_table,
> +  &xgene1_regmove_cost,
> +  &xgene1_vector_cost,
> +  &generic_branch_cost,
> +  &xgene1_approx_modes,
> +  6, /* memmov_cost  */
> +  4, /* issue_rate  */
> +  AARCH64_FUSE_NOTHING, /* fusible_ops  */
> +  "16",	/* function_align.  */
> +  "16",	/* jump_align.  */
> +  "16",	/* loop_align.  */
> +  2,	/* int_reassoc_width.  */
> +  4,	/* fp_reassoc_width.  */
> +  1,	/* vec_reassoc_width.  */
> +  2,	/* min_div_recip_mul_sf.  */
> +  2,	/* min_div_recip_mul_df.  */
> +  17,	/* max_case_values.  */
> +  tune_params::AUTOPREFETCHER_OFF,	/* autoprefetcher_model.  */
> +  (AARCH64_EXTRA_TUNE_NO_LDP_STP_QREGS),	/* tune_flags.  */
> +  &xgene1_prefetch_tune
> +};
> +
>   static const struct tune_params qdf24xx_tunings =
>   {
>     &qdf24xx_extra_costs,
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index e016dce..ac81fb2 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -15288,7 +15288,7 @@ Specify the name of the target processor for which GCC should tune the
>   performance of the code.  Permissible values for this option are:
>   @samp{generic}, @samp{cortex-a35}, @samp{cortex-a53}, @samp{cortex-a55},
>   @samp{cortex-a57}, @samp{cortex-a72}, @samp{cortex-a73}, @samp{cortex-a75},
> -@samp{cortex-a76}, @samp{ares}, @samp{exynos-m1}, @samp{falkor},
> +@samp{cortex-a76}, @samp{ares}, @samp{exynos-m1}, @samp{emag}, @samp{falkor},
>   @samp{qdf24xx}, @samp{saphira}, @samp{phecda}, @samp{xgene1}, @samp{vulcan},
>   @samp{thunderx}, @samp{thunderxt88}, @samp{thunderxt88p1}, @samp{thunderxt81},
>   @samp{tsv110}, @samp{thunderxt83}, @samp{thunderx2t99},