Re: [PATCH] [ARM] Add support for the Samsung Exynos M1 processor

[Kyrill Tkachov <kyrylo.tkachov@arm.com>]

Hi Evandro
On 30/03/15 22:51, Evandro Menezes wrote:
> The Samsung Exynos M1 implements the ARMv8 ISA and this patch adds support
> for it through the -mcpu command-line option.
>
> The patch was checked on arm-unknown-linux-gnueabihf without new failures.
>
> OK for trunk?
>
> -- Evandro Menezes Austin, TX
>
> 0001-ARM-Add-option-for-the-Samsung-Exynos-M1-core-for-AR.patch
>
>
> diff --git a/gcc/config/arm/arm-cores.def b/gcc/config/arm/arm-cores.def
> index b22ea7f..0710a38 100644
> --- a/gcc/config/arm/arm-cores.def
> +++ b/gcc/config/arm/arm-cores.def
> @@ -168,6 +168,7 @@ ARM_CORE("cortex-a17.cortex-a7", cortexa17cortexa7, cortexa7,	7A,  FL_LDSCHED |
>   ARM_CORE("cortex-a53",	cortexa53, cortexa53,	8A, FL_LDSCHED | FL_CRC32, cortex_a53)
>   ARM_CORE("cortex-a57",	cortexa57, cortexa57,	8A, FL_LDSCHED | FL_CRC32, cortex_a57)
>   ARM_CORE("cortex-a72",	cortexa72, cortexa57,	8A, FL_LDSCHED | FL_CRC32, cortex_a57)
> +ARM_CORE("exynos-m1",	exynosm1,  exynosm1,	8A, FL_LDSCHED | FL_CRC32, exynosm1)

There are two problems with this:
* The 3rd field of ARM_CORE represents the scheduling identifier and 
without a
separate pipeline description for exynosm1 this will just use the 
generic_sched
scheduler which performs quite poorly on modern cores.  Would you prefer to
reuse a pipeline description from one of the pre-existing ones? Look for 
example
at the cortex-a72 definition:
ARM_CORE("cortex-a72",    cortexa72, cortexa57,  <...snip>
here the cortexa57 means 'make scheduling decisions for cortexa57'.

* The final field in ARM_CORE specifies the tuning struct to be used for 
this core.
This should be defined in arm.c and have the form 'arm_<ident>_tune, so 
for your
case it should be arm_exynosm1_tune. This isn't defined in your patch, 
so it won't
compile without that. You can write a custom tuning struct yourself, or 
reuse a
tuning struct for one of the existing cores, if you'd like.

Also, you should add exynosm1 to the switch statement in arm_issue_rate 
to specify
the issue rate. I have a patch for next stage1 that should refactor it 
all into the
tuning structs 
(https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02706.html) but until
that goes in, you should fill in the switch statement there.

Thanks,
Kyrill