[OE-core] [PATCH] arch-arm: Do not add -march options for arm architecture

Fri Jan 25 03:22:44 UTC 2019

On Wed, Jan 23, 2019 at 6:11 PM Andre McCurdy <armccurdy at gmail.com> wrote:
>
> On Wed, Jan 23, 2019 at 2:50 PM Khem Raj <raj.khem at gmail.com> wrote:
> >
> > On Wed, Jan 23, 2019 at 5:27 PM Andre McCurdy <armccurdy at gmail.com> wrote:
> > >
> > > On Wed, Jan 23, 2019 at 12:05 PM Khem Raj <raj.khem at gmail.com> wrote:
> > > >
> > > > tune files which inherit the arch definitions already define appropriate
> > > > -mcpu option, which is equivalent of right -march and -mtune combination
> > >
> > > And what about machines which inherit an arch definition instead of a
> > > cpu definition? Is that no longer supported?
> >
> > I could not find such a machine configuration in several BSP layers I looked at.
> > Do you know of machine definitions where these files are included
> > directly in machine configs ?
>
> Using a CPU specific include but then leaving the generic DEFAULTTUNE
> (ie not enabling any CPU specific TUNE_CCARGS) is equivalent, and I
> guess quite common.

defaulttune does not play much part in appending -mcpu option to
compiler flags, A case
where this could bypass the setting mcpu is when we do not include a
tune-* for arm based machine
and thats the case I was interested to know.

again if there is such a BSP I would be interested to test it out.
>
> > if so, then we have to either use _remove or introduce a weak variable
> > to set march only if
> > mcpu is not set which
>
> I think the solution is to ensure that the CPU specific tuning options
> are always compatible with any architecture specific options which may
> be active.
>
> In general our CPU specific tuning files are a little sloppy (e.g.
> defaulting to combinations of VFP versions and architecture levels
> which never exist in the real world). If newer versions of gcc are
> making those issues more apparent then why not take the opportunity to
> fix them properly?

the mcpu option is helping with right march/mtune combination. while we can
use specific -march=arch(+-)features syntax and combine with right
-mtune to match -mcpu I think we should use
compiler defaults and not try to recreate something that -mcpu would
do it easily for us.

So either we drop the idea of setting mcpu and use basic -march value
combined with -mtune ( which btw. was default in past)
or if we want to let compiler use ISA for a particular arch
implementation ( cortexXXX ) then we should just use -mcpu and drop
using -mtune and -march.

I would include towards option 1.

>
> > > > and is preferred since gcc is getting stricter and stricter with option
> > > > check semantics and can now find incompatible -march and -mcpu options
> > > > better with every release. It does internal feature consistency check
> > > > and if it finds out discrepency between what -mcpu would expand to as
> > > > compared to -march it will flag the options to be incompatible, for
> > > > naked eye it sounds wrong but gcc would translate -mcpu to a given
> > > > -march internally and it might not match to what we set in these arch
> > > > files.
> > > >
> > > > The effects are quite subtle, where this can result in configure test
> > > > failing to compile due to these incompatible options and a feature
> > > > option getting disabled for a recipe for no reason.
> > > >
> > > > e.g. with gcc9 which can now detect that -mcpu=cortex-a5 and
> > > > -march=armv7-a are incompatible, many features in libstdc++ ends up
> > > > disabled due to configure check failures e.g. size_t size, ptrdiff_t
> > > > sizes, which inturn results in compiling libstdc++ with unwanted
> > > > disabled features.
> > >
> > > It would be interesting to see more specific details of this.
> >
> > These are configure tests which fail when -Werror is in use and is not
> > limited to libstdc++
> > it can happen in any package. Since the diagnostic about incompatible
> > march mcpu pair
> > is a warning in general, it does not matter, but when we have -Werror
> > enabled this turns
> > into error, You can simply try a small helloworld example to see the effects.
> >
> > >
> > > > Signed-off-by: Khem Raj <raj.khem at gmail.com>
> > > > ---
> > > >  meta/conf/machine/include/arm/arch-armv4.inc   | 1 -
> > > >  meta/conf/machine/include/arm/arch-armv5.inc   | 1 -
> > > >  meta/conf/machine/include/arm/arch-armv6.inc   | 1 -
> > > >  meta/conf/machine/include/arm/arch-armv7a.inc  | 1 -
> > > >  meta/conf/machine/include/arm/arch-armv7ve.inc | 1 -
> > > >  meta/conf/machine/include/tune-iwmmxt.inc      | 2 +-
> > > >  6 files changed, 1 insertion(+), 6 deletions(-)
> > > >
> > > > diff --git a/meta/conf/machine/include/arm/arch-armv4.inc b/meta/conf/machine/include/arm/arch-armv4.inc
> > > > index 47a7ad2830..52d8ab1e8f 100644
> > > > --- a/meta/conf/machine/include/arm/arch-armv4.inc
> > > > +++ b/meta/conf/machine/include/arm/arch-armv4.inc
> > > > @@ -2,7 +2,6 @@ DEFAULTTUNE ?= "armv4"
> > > >
> > > >  TUNEVALID[arm] = "Enable ARM instruction set"
> > > >  TUNEVALID[armv4] = "Enable instructions for ARMv4"
> > > > -TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv4', ' -march=armv4t', '', d)}"
> > > >  # enable --fix-v4bx when we have armv4 in TUNE_FEATURES, but then disable it when we have also armv5 or thumb
> > > >  # maybe we should extend bb.utils.contains to support check for any checkvalues in value, now it does
> > > >  # checkvalues.issubset(val) which cannot be used for negative test of foo neither bar in value
> > > > diff --git a/meta/conf/machine/include/arm/arch-armv5.inc b/meta/conf/machine/include/arm/arch-armv5.inc
> > > > index f9068af9de..1fe1b6b8e4 100644
> > > > --- a/meta/conf/machine/include/arm/arch-armv5.inc
> > > > +++ b/meta/conf/machine/include/arm/arch-armv5.inc
> > > > @@ -2,7 +2,6 @@ DEFAULTTUNE ?= "armv5"
> > > >
> > > >  TUNEVALID[armv5] = "Enable instructions for ARMv5"
> > > >  TUNECONFLICTS[armv5] = "armv4"
> > > > -TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv5', ' -march=armv5t${ARMPKGSFX_DSP}', '', d)}"
> > > >  MACHINEOVERRIDES =. "${@bb.utils.contains('TUNE_FEATURES', 'armv5', 'armv5:', '' ,d)}"
> > > >
> > > >  require conf/machine/include/arm/arch-armv4.inc
> > > > diff --git a/meta/conf/machine/include/arm/arch-armv6.inc b/meta/conf/machine/include/arm/arch-armv6.inc
> > > > index 6c838e999c..adb9be8050 100644
> > > > --- a/meta/conf/machine/include/arm/arch-armv6.inc
> > > > +++ b/meta/conf/machine/include/arm/arch-armv6.inc
> > > > @@ -2,7 +2,6 @@ DEFAULTTUNE ?= "armv6hf"
> > > >
> > > >  TUNEVALID[armv6] = "Enable instructions for ARMv6"
> > > >  TUNECONFLICTS[armv6] = "armv4 armv5"
> > > > -TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv6', ' -march=armv6', '', d)}"
> > > >  MACHINEOVERRIDES =. "${@bb.utils.contains('TUNE_FEATURES', 'armv6', 'armv6:', '' ,d)}"
> > > >
> > > >  require conf/machine/include/arm/arch-armv5-dsp.inc
> > > > diff --git a/meta/conf/machine/include/arm/arch-armv7a.inc b/meta/conf/machine/include/arm/arch-armv7a.inc
> > > > index a2663d8008..09d2c03a5d 100644
> > > > --- a/meta/conf/machine/include/arm/arch-armv7a.inc
> > > > +++ b/meta/conf/machine/include/arm/arch-armv7a.inc
> > > > @@ -3,7 +3,6 @@ ARM_INSTRUCTION_SET ?= "thumb"
> > > >
> > > >  TUNEVALID[armv7a] = "Enable instructions for ARMv7-a"
> > > >  TUNECONFLICTS[armv7a] = "armv4 armv5 armv6 armv7"
> > > > -TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv7a', ' -march=armv7-a', '', d)}"
> > > >  MACHINEOVERRIDES =. "${@bb.utils.contains('TUNE_FEATURES', 'armv7a', 'armv7a:', '' ,d)}"
> > > >
> > > >  require conf/machine/include/arm/arch-armv6.inc
> > > > diff --git a/meta/conf/machine/include/arm/arch-armv7ve.inc b/meta/conf/machine/include/arm/arch-armv7ve.inc
> > > > index 4d9260fecb..31e334f645 100644
> > > > --- a/meta/conf/machine/include/arm/arch-armv7ve.inc
> > > > +++ b/meta/conf/machine/include/arm/arch-armv7ve.inc
> > > > @@ -2,7 +2,6 @@ DEFAULTTUNE ?= "armv7vethf"
> > > >
> > > >  TUNEVALID[armv7ve] = "Enable instructions for ARMv7ve"
> > > >  TUNECONFLICTS[armv7ve] = "armv4 armv5 armv6 armv7 armv7a"
> > > > -TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'armv7ve', ' -march=armv7ve', '', d)}"
> > > >  MACHINEOVERRIDES =. "${@bb.utils.contains('TUNE_FEATURES', 'armv7ve', 'armv7ve:', '' ,d)}"
> > > >
> > > >  require conf/machine/include/arm/arch-armv7a.inc
> > > > diff --git a/meta/conf/machine/include/tune-iwmmxt.inc b/meta/conf/machine/include/tune-iwmmxt.inc
> > > > index f27423cb2e..6e577697cc 100644
> > > > --- a/meta/conf/machine/include/tune-iwmmxt.inc
> > > > +++ b/meta/conf/machine/include/tune-iwmmxt.inc
> > > > @@ -6,7 +6,7 @@ DEFAULTTUNE ?= "iwmmxt"
> > > >  require conf/machine/include/arm/arch-armv5-dsp.inc
> > > >
> > > >  TUNEVALID[iwmmxt] = "Enable Intel PXA27x specific processor optimizations"
> > > > -TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'iwmmxt', ' -march=iwmmxt -mcpu=iwmmxt', '', d)}"
> > > > +TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'iwmmxt', ' -mcpu=iwmmxt', '', d)}"
> > > >
> > > >  AVAILTUNES += "iwmmxt"
> > > >  ARMPKGARCH_tune-iwmmxt = "iwmmxt"
> > > > --
> > > > 2.20.1
> > > >
> > > > --
> > > > _______________________________________________
> > > > Openembedded-core mailing list
> > > > Openembedded-core at lists.openembedded.org
> > > > http://lists.openembedded.org/mailman/listinfo/openembedded-core