On Wed, 2020-09-09 at 19:21 -0400, Jon Mason wrote:
> On Wed, Sep 9, 2020 at 6:59 PM Mark Hatle
> <mark.ha...@kernel.crashing.org> wrote:
> > I like the direction of this work, but one comment.. (down below)
> >
> > On 9/9/20 5:45 PM, Jon Mason wrote:
> > > Migrate the settings in tune-cortexa55.inc to arch-armv8-
> > > 2a.inc. This
> > > will allow for a single file to include all of the tunings of a
> > > family
> > > of processors. This will reduce the proliferation of unique
> > > files per
> > > processor currently occuring in conf/machine/include/
> > >
> > > Signed-off-by: Jon Mason <jon.ma...@arm.com>
> > > ---
> > > .../machine/include/arm/arch-armv8-2a.inc | 31
> > > ++++++++++++++-----
> > > meta/conf/machine/include/tune-cortexa55.inc | 12 -------
> > > 2 files changed, 23 insertions(+), 20 deletions(-)
> > > delete mode 100644 meta/conf/machine/include/tune-cortexa55.inc
> > >
> > > diff --git a/meta/conf/machine/include/arm/arch-armv8-2a.inc
> > > b/meta/conf/machine/include/arm/arch-armv8-2a.inc
> > > index b40ebf176e43..38564a17d98b 100644
> > > --- a/meta/conf/machine/include/arm/arch-armv8-2a.inc
> > > +++ b/meta/conf/machine/include/arm/arch-armv8-2a.inc
> > > @@ -1,4 +1,19 @@
> > > -DEFAULTTUNE ?= "armv8-2a"
> > > +#
> > > +# Tune Settings for Cortex-A55
> > > +#
> > > +TUNEVALID[cortexa55] = "Enable Cortex-A55 specific processor
> > > optimizations"
> > > +TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES',
> > > 'cortexa55', ' -mcpu=cortex-a55', '', d)}"
> > > +
> > > +# Little Endian base configs
> > > +AVAILTUNES +=
> > > "cortexa55"
> > > +ARMPKGARCH_tune-cortexa55 =
> > > "cortexa55"
> > > +TUNE_FEATURES_tune-cortexa55 = "aarch64
> > > cortexa55 crypto"
> > > +PACKAGE_EXTRA_ARCHS_tune-cortexa55 =
> > > "${PACKAGE_EXTRA_ARCHS_tune-armv8-2a-crypto} cortexa55"
> > > +
> > > +#
> > > +# Defaults for ARMv8-a
> > > +#
> > > +DEFAULTTUNE ?= "armv8-2a"
> > >
> > > TUNEVALID[armv8-2a] = "Enable instructions for ARMv8-a"
> > > TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv8-
> > > 2a', ' -march=armv8.2-a', '', d)}"
> > > @@ -8,10 +23,10 @@ MACHINEOVERRIDES =. "${@bb.utils.contains('TUN
> > > E_FEATURES', 'armv8-2a', 'armv8-2a
> > > require conf/machine/include/arm/arch-armv8a.inc
> > >
> > > # Little Endian base configs
> > > -AVAILTUNES += "armv8-2a armv8-2a-crypto"
> > > -ARMPKGARCH_tune-armv8-2a ?= "armv8-2a"
> > > -ARMPKGARCH_tune-armv8-2a-crypto ?= "armv8-2a"
> > > -TUNE_FEATURES_tune-armv8-2a = "aarch64 armv8-
> > > 2a"
> > > -TUNE_FEATURES_tune-armv8-2a-crypto =
> > > "${TUNE_FEATURES_tune-armv8-2a} crypto"
> > > -PACKAGE_EXTRA_ARCHS_tune-armv8-2a =
> > > "${PACKAGE_EXTRA_ARCHS_tune-armv8a} armv8-2a"
> > > -PACKAGE_EXTRA_ARCHS_tune-armv8-2a-crypto =
> > > "${PACKAGE_EXTRA_ARCHS_tune-armv8-2a} armv8-2a-crypto"
> > > +AVAILTUNES += "armv8-2a
> > > armv8-2a-crypto"
> > > +ARMPKGARCH_tune-armv8-2a ?= "armv8-2a"
> > > +ARMPKGARCH_tune-armv8-2a-crypto ?= "armv8-2a"
> > > +TUNE_FEATURES_tune-armv8-2a = "aarch64
> > > armv8-2a"
> > > +TUNE_FEATURES_tune-armv8-2a-crypto =
> > > "${TUNE_FEATURES_tune-armv8-2a} crypto"
> > > +PACKAGE_EXTRA_ARCHS_tune-armv8-2a =
> > > "${PACKAGE_EXTRA_ARCHS_tune-armv8a} armv8-2a"
> > > +PACKAGE_EXTRA_ARCHS_tune-armv8-2a-crypto =
> > > "${PACKAGE_EXTRA_ARCHS_tune-armv8-2a} armv8-2a-crypto"
> >
> > It may make sense, instead of deleting it, to have just the
> > 'require
> > conf/machine/include/arm/arch-armv8-2a.inc' file for
> > transition. (If a
> > transition period is NOT required or desired, then this isn't
> > necessary!)
> >
> > Tricky part is how to notify someone they're in the midst of the
> > transition
> > period, and using the deprecated behavior.
> >
> > (In past work, I've had a class that looked for a variable to
> > exist, if it did
> > it printed the contents as warning messages to the console after
> > parsing. I'd
> > love to see something like this as standard behavior for future
> > cases like this.)
>
> Fantastic idea. I hate the idea of breaking backward compat. Any
> newly added tunes would not need this, so no extra files for my new
> 13.
>
> Question 1. How can we do this warning? Is there something I can
> steal from as a basis?
> Question 2. How long do we keep this compat around?
I actually lean the other way. By removing the file you make it clear
to the user they have to do something. Adding in the compat in this
case doesn't even show a warning, so we just kick the can down the
road.
More recently I have lent towards "hard" fails which force the user
into adapting to the new situation as otherwise I think we're kind of
giving false promises...
Cheers,
Richard
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#142356):
https://lists.openembedded.org/g/openembedded-core/message/142356
Mute This Topic: https://lists.openembedded.org/mt/76744241/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
