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

[Evandro Menezes <e dot menezes at samsung dot com>]

Hi, Kyrill.

At this moment, it suffices to use the same scheduling as Cortex A57, but
more specific details are to be expected.

I couldn't check the build though, as my Arndale is strange today.  As soon
as it's healthy, I'll check it.  

I appreciate your feedback.

-- 
Evandro Menezes                              Austin, TX


> -----Original Message-----
> From: Kyrill Tkachov [mailto:kyrylo.tkachov@arm.com]
> Sent: Tuesday, March 31, 2015 3:33
> To: Evandro Menezes; 'GCC Patches'
> Subject: Re: [PATCH] [ARM] Add support for the Samsung Exynos M1 processor
> 
> 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

Attachment: 0001-ARM-Add-option-for-the-Samsung-Exynos-M1-core.patch
Description: Binary data