[Buildroot] [PATCH 0/1] Fix redundant usage of -mcpu and -march/-mtune

[Buildroot] [PATCH 0/1] Fix redundant usage of -mcpu and -march/-mtune

[Karoly Kasza]

Hi,

I've been using the Linaro external toolchain in the latest GIT to generate ARM
code. I've found that when using this external compiler, Buildroot
compulsorily specifies the -mcpu and -march switches to GCC, which is redundant
(-march and -mtune can be concluded from -mcpu, see
http://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html )
While most of the time this only generates a warning, mmc-utils can not be
compiled this way and probably other packages suffer from this as well.

I recommend altering toolchain/toolchain-external/toolchain-external.mk,
so it won't supply redundant GCC switches.

Regards,
Karoly

Karoly Kasza (1):
  Fix redundant usage of -mcpu and -march/-mtune

 toolchain/toolchain-external/toolchain-external.mk |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

-- 
1.7.10.4

[Buildroot] [PATCH 1/1] Fix redundant usage of -mcpu and -march/-mtune

[Karoly Kasza]

Disable adding -mtune or -march to the external toolchain parameters
if -mcpu is present.

Signed-off-by: Karoly Kasza <kaszak@gmail.com>
---
 toolchain/toolchain-external/toolchain-external.mk |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
index 3f88188..9ce4a2f 100644
--- a/toolchain/toolchain-external/toolchain-external.mk
+++ b/toolchain/toolchain-external/toolchain-external.mk
@@ -181,6 +181,10 @@ ifeq ($(BR2_x86_64),y)
 TOOLCHAIN_EXTERNAL_CFLAGS += -m64
 TOOLCHAIN_EXTERNAL_WRAPPER_ARGS += -DBR_64
 endif
+ifneq ($(CC_TARGET_CPU_),)
+TOOLCHAIN_EXTERNAL_CFLAGS += -mcpu=$(CC_TARGET_CPU_)
+TOOLCHAIN_EXTERNAL_WRAPPER_ARGS += -DBR_CPU='"$(CC_TARGET_CPU_)"'
+else
 ifneq ($(CC_TARGET_TUNE_),)
 TOOLCHAIN_EXTERNAL_CFLAGS += -mtune=$(CC_TARGET_TUNE_)
 TOOLCHAIN_EXTERNAL_WRAPPER_ARGS += -DBR_TUNE='"$(CC_TARGET_TUNE_)"'
@@ -189,9 +193,6 @@ ifneq ($(CC_TARGET_ARCH_),)
 TOOLCHAIN_EXTERNAL_CFLAGS += -march=$(CC_TARGET_ARCH_)
 TOOLCHAIN_EXTERNAL_WRAPPER_ARGS += -DBR_ARCH='"$(CC_TARGET_ARCH_)"'
 endif
-ifneq ($(CC_TARGET_CPU_),)
-TOOLCHAIN_EXTERNAL_CFLAGS += -mcpu=$(CC_TARGET_CPU_)
-TOOLCHAIN_EXTERNAL_WRAPPER_ARGS += -DBR_CPU='"$(CC_TARGET_CPU_)"'
 endif
 ifneq ($(CC_TARGET_ABI_),)
 TOOLCHAIN_EXTERNAL_CFLAGS += -mabi=$(CC_TARGET_ABI_)
-- 
1.7.10.4

[Buildroot] [PATCH 1/1] Fix redundant usage of -mcpu and -march/-mtune

[Thomas Petazzoni]

Dear Karoly Kasza,

On Tue, 13 May 2014 15:58:14 +0200, Karoly Kasza wrote:
> Disable adding -mtune or -march to the external toolchain parameters
> if -mcpu is present.
> 
> Signed-off-by: Karoly Kasza <kaszak@gmail.com>
> ---
>  toolchain/toolchain-external/toolchain-external.mk |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

We discussed this patch at the last Buildroot meeting, and our feeling
is that it's not the correct approach. I've since then sent a patch
series that fix the same problem. Our approach is:

 * On ARM, to no longer specify -march, but only -mcpu, since the
   latter is used by gcc to deduce the former.

 * In general, get rid of -mtune, since it doesn't make much sense in
   the context of Buildroot where we target one specific system, and
   therefore using -mcpu is sufficient.

So, we've marked your patch as Rejected in our patch tracking system.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] Fix redundant usage of -mcpu and -march/-mtune

[Károly Kasza]

Hi Thomas,

thanks for the notice, I've seen the changes since then.

Best regards,
Karoly

On Sat, Nov 1, 2014 at 10:22 PM, Thomas Petazzoni <
thomas.petazzoni@free-electrons.com> wrote:

> Dear Karoly Kasza,
>
> On Tue, 13 May 2014 15:58:14 +0200, Karoly Kasza wrote:
> > Disable adding -mtune or -march to the external toolchain parameters
> > if -mcpu is present.
> >
> > Signed-off-by: Karoly Kasza <kaszak@gmail.com>
> > ---
> >  toolchain/toolchain-external/toolchain-external.mk |    7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
>
> We discussed this patch at the last Buildroot meeting, and our feeling
> is that it's not the correct approach. I've since then sent a patch
> series that fix the same problem. Our approach is:
>
>  * On ARM, to no longer specify -march, but only -mcpu, since the
>    latter is used by gcc to deduce the former.
>
>  * In general, get rid of -mtune, since it doesn't make much sense in
>    the context of Buildroot where we target one specific system, and
>    therefore using -mcpu is sufficient.
>
> So, we've marked your patch as Rejected in our patch tracking system.
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
>



-- 

?dv,
KK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20141101/604084a2/attachment.html>

[Buildroot] [PATCH 0/1] Fix redundant usage of -mcpu and -march/-mtune

[Thomas Petazzoni]

Dear Karoly Kasza,

Thanks for your report!

On Tue, 13 May 2014 15:58:13 +0200, Karoly Kasza wrote:

> I've been using the Linaro external toolchain in the latest GIT to generate ARM
> code. I've found that when using this external compiler, Buildroot
> compulsorily specifies the -mcpu and -march switches to GCC, which is redundant
> (-march and -mtune can be concluded from -mcpu, see
> http://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html )
> While most of the time this only generates a warning, mmc-utils can not be
> compiled this way and probably other packages suffer from this as well.

Which mmc-utils problem have you seen? I've just tried building the
following configuration:

BR2_arm=y
BR2_cortex_a8=y
BR2_ARM_EABIHF=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_PACKAGE_MMC_UTILS=y

And it just built fine, and uses a Linaro toolchain.

> I recommend altering toolchain/toolchain-external/toolchain-external.mk,
> so it won't supply redundant GCC switches.

The potential problem I see is that the code you're changing is used
for *all* architectures, but the change you're making has only been
made with ARM specificities in mind. Does ignoring -march and -mtune
when -mcpu is defined also works for all other architectures?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 0/1] Fix redundant usage of -mcpu and -march/-mtune

[Károly Kasza]

Hi Thomas,

On Tue, May 13, 2014 at 4:04 PM, Thomas Petazzoni <
thomas.petazzoni@free-electrons.com> wrote:

> Dear Karoly Kasza,
>
> Thanks for your report!
>
> On Tue, 13 May 2014 15:58:13 +0200, Karoly Kasza wrote:
>
> > I've been using the Linaro external toolchain in the latest GIT to
> generate ARM
> > code. I've found that when using this external compiler, Buildroot
> > compulsorily specifies the -mcpu and -march switches to GCC, which is
> redundant
> > (-march and -mtune can be concluded from -mcpu, see
> > http://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html )
> > While most of the time this only generates a warning, mmc-utils can not
> be
> > compiled this way and probably other packages suffer from this as well.
>
> Which mmc-utils problem have you seen? I've just tried building the
> following configuration:
>
> BR2_arm=y
> BR2_cortex_a8=y
> BR2_ARM_EABIHF=y
> BR2_TOOLCHAIN_EXTERNAL=y
> BR2_PACKAGE_MMC_UTILS=y
>
> And it just built fine, and uses a Linaro toolchain.
>
>
The BR2_cortex_a7=y breaks it, it will generate a warning while compiling
busybox (every invocation of gcc):
warning: switch -mcpu=cortex-a7 conflicts with -march=armv7-a switch
[enabled by default]

Then an error with mmc-utils like this:
mmc.c:1:0: error: switch -mcpu=cortex-a7 conflicts with -march=armv7-a
switch [-Werror]


On second thought, the above problem may come from when defining an -march
that is the same as -mcpu it will generate a warning, but it won't if these
two differ. In buildroot/arch/Config.in.arm BR2_GCC_TARGET_ARCH (from which
-march is derived) is always defined as armv7-a when using any Cortex CPU
(because Linaro always generates armv7 code?).
So -march should be ignored when -mcpu matches it (for example armv7 and
Cortex-a7) but it may be needed to be defined for different -mcpu -s.


> I recommend altering toolchain/toolchain-external/toolchain-external.mk,
> > so it won't supply redundant GCC switches.
>
> The potential problem I see is that the code you're changing is used
> for *all* architectures, but the change you're making has only been
> made with ARM specificities in mind. Does ignoring -march and -mtune
> when -mcpu is defined also works for all other architectures?
>
>
This is a tough question, because based on the GCC online manual, not all
architectures accept all three parameters.
It looks like -mcpu is always the most definite, but is is deprecated on
i386 and x86_64 for example.

Maybe it would be wise to define and use these 3 parameters only based on
the target architecture.


Regards,
Karoly


> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20140513/1f74c876/attachment.html>

[Buildroot] [PATCH 0/1] Fix redundant usage of -mcpu and -march/-mtune

[Thomas Petazzoni]

Dear K?roly Kasza,

On Tue, 13 May 2014 16:33:18 +0200, K?roly Kasza wrote:

> The BR2_cortex_a7=y breaks it, it will generate a warning while compiling
> busybox (every invocation of gcc):
> warning: switch -mcpu=cortex-a7 conflicts with -march=armv7-a switch
> [enabled by default]
> 
> Then an error with mmc-utils like this:
> mmc.c:1:0: error: switch -mcpu=cortex-a7 conflicts with -march=armv7-a
> switch [-Werror]

Indeed, with Cortex-A7, I can reproduce this. This is because
Cortex-A7, A-12 and A-15 support more features than the other armv7-a
cores (A5, A8, A9), so gcc has a separate -march argument: armv7ve.

So normally, we should set BR2_GCC_TARGET_ARCH to armv7ve for those
Cortex-A variants. I'm currently experimenting with this, but I'm
having a few issues. Also, it's likely that armv7ve is quite recent,
and may not be present in older gcc versions.

> > The potential problem I see is that the code you're changing is used
> > for *all* architectures, but the change you're making has only been
> > made with ARM specificities in mind. Does ignoring -march and -mtune
> > when -mcpu is defined also works for all other architectures?
> >
> This is a tough question, because based on the GCC online manual, not all
> architectures accept all three parameters.
> It looks like -mcpu is always the most definite, but is is deprecated on
> i386 and x86_64 for example.
> 
> Maybe it would be wise to define and use these 3 parameters only based on
> the target architecture.

This is already the case. They are only passed if
BR2_GCC_TARGET_{ARCH,CPU,TUNE} are defined. Some architectures (such as
ARM) defines all three, but other architectures (such as x86) do not.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 0/1] Fix redundant usage of -mcpu and -march/-mtune

[Károly Kasza]

Dear Thomas


On Tue, May 13, 2014 at 10:22 PM, Thomas Petazzoni <
thomas.petazzoni@free-electrons.com> wrote:

> Dear K?roly Kasza,
>
> On Tue, 13 May 2014 16:33:18 +0200, K?roly Kasza wrote:
>
> > The BR2_cortex_a7=y breaks it, it will generate a warning while compiling
> > busybox (every invocation of gcc):
> > warning: switch -mcpu=cortex-a7 conflicts with -march=armv7-a switch
> > [enabled by default]
> >
> > Then an error with mmc-utils like this:
> > mmc.c:1:0: error: switch -mcpu=cortex-a7 conflicts with -march=armv7-a
> > switch [-Werror]
>
> Indeed, with Cortex-A7, I can reproduce this. This is because
> Cortex-A7, A-12 and A-15 support more features than the other armv7-a
> cores (A5, A8, A9), so gcc has a separate -march argument: armv7ve.
>
> So normally, we should set BR2_GCC_TARGET_ARCH to armv7ve for those
> Cortex-A variants. I'm currently experimenting with this, but I'm
> having a few issues. Also, it's likely that armv7ve is quite recent,
> and may not be present in older gcc versions.
>
>
That's the case with the Linaro 2014.02 toolchain - in GCC 4.8.2 the
armv7ve does not exist
(http://gcc.gnu.org/onlinedocs/gcc-4.8.2/gcc/ARM-Options.html#ARM-Options).


> > > The potential problem I see is that the code you're changing is used
> > > for *all* architectures, but the change you're making has only been
> > > made with ARM specificities in mind. Does ignoring -march and -mtune
> > > when -mcpu is defined also works for all other architectures?
> > >
> > This is a tough question, because based on the GCC online manual, not all
> > architectures accept all three parameters.
> > It looks like -mcpu is always the most definite, but is is deprecated on
> > i386 and x86_64 for example.
> >
> > Maybe it would be wise to define and use these 3 parameters only based on
> > the target architecture.
>
> This is already the case. They are only passed if
> BR2_GCC_TARGET_{ARCH,CPU,TUNE} are defined. Some architectures (such as
> ARM) defines all three, but other architectures (such as x86) do not.
>
> Now I see. In this case, maybe the simplest would be to just leave
BR2_GCC_TARGET_ARCH
undefined in arch/Config.in.arm if the user selected a proper CPU (and not
just some generic one).

Also, based on the documentation it should be completely unnecessary to
define -march if we
have the accurate CPU with -mcpu (on ARM at least).


> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
>

Regards,
Karoly
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20140514/cd5221cb/attachment.html>

[Buildroot] [PATCH 0/1] Fix redundant usage of -mcpu and -march/-mtune

[Thomas Petazzoni]

Dear K?roly Kasza,

On Wed, 14 May 2014 09:55:09 +0200, K?roly Kasza wrote:

> > So normally, we should set BR2_GCC_TARGET_ARCH to armv7ve for those
> > Cortex-A variants. I'm currently experimenting with this, but I'm
> > having a few issues. Also, it's likely that armv7ve is quite recent,
> > and may not be present in older gcc versions.
> >
> >
> That's the case with the Linaro 2014.02 toolchain - in GCC 4.8.2 the
> armv7ve does not exist
> (http://gcc.gnu.org/onlinedocs/gcc-4.8.2/gcc/ARM-Options.html#ARM-Options).

Yeah, in gcc 4.8.2 vanilla. But Linaro toolchains don't use a vanilla
gcc, they have lots of ARM related patches. And if the Linaro
toolchains complains that -march=armv7-a is incompatible with
-mcpu=cortex-a7, then quite certainly it means that cortex-a7 is part
of a different -march family, according to this toolchain.

> > This is already the case. They are only passed if
> > BR2_GCC_TARGET_{ARCH,CPU,TUNE} are defined. Some architectures (such as
> > ARM) defines all three, but other architectures (such as x86) do not.
> >
> Now I see. In this case, maybe the simplest would be to just leave
> BR2_GCC_TARGET_ARCH
> undefined in arch/Config.in.arm if the user selected a proper CPU (and not
> just some generic one).

Not sure what you mean by "not just some generic one". In the
menuconfig, what we ask the user is to define the specific ARM core
being used. From that we can derive the -march and -mcpu values. But
indeed, the -march value is quite useless.

However, I believe there are place in Buildroot where we do use the
BR2_GCC_TARGET_ARCH value for other things than just the compiler
flags. We would have to audit those before removing BR2_GCC_TARGET_ARCH
from Config.in.arm.

> Also, based on the documentation it should be completely unnecessary to
> define -march if we have the accurate CPU with -mcpu (on ARM at least).

Correct.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 0/1] Fix redundant usage of -mcpu and -march/-mtune

[Károly Kasza]

Dear Thomas,

On Wed, May 14, 2014 at 9:59 AM, Thomas Petazzoni <
thomas.petazzoni@free-electrons.com> wrote:

> Dear K?roly Kasza,
>
> On Wed, 14 May 2014 09:55:09 +0200, K?roly Kasza wrote:
>
> > >
> > That's the case with the Linaro 2014.02 toolchain - in GCC 4.8.2 the
> > armv7ve does not exist
> > (
> http://gcc.gnu.org/onlinedocs/gcc-4.8.2/gcc/ARM-Options.html#ARM-Options).
>
> Yeah, in gcc 4.8.2 vanilla. But Linaro toolchains don't use a vanilla
> gcc, they have lots of ARM related patches. And if the Linaro
> toolchains complains that -march=armv7-a is incompatible with
> -mcpu=cortex-a7, then quite certainly it means that cortex-a7 is part
> of a different -march family, according to this toolchain.
>
>
I ran gcc-linaro-arm-linux-gnueabihf-4.8-2014.02_linux#
./bin/arm-linux-gnueabihf-gcc --target-help

It says:

...

 Known ARM architectures (for use with the -march= option):
    armv2 armv2a armv3 armv3m armv4 armv4t armv5 armv5e armv5t armv5te armv6
    armv6-m armv6j armv6k armv6s-m armv6t2 armv6z armv6zk armv7 armv7-a
armv7-m
    armv7-r armv7e-m armv8-a armv8-a+crc iwmmxt iwmmxt2 native
...

So it looks like it does not currently support armv7-ve. I don't know what
-march does -mcpu=cortex-a7 define, if not armv7-a.
There is a newer Linaro toolchain (2014.04), which is not yet in the
Buildroot GIT. It is still based on GCC 4.8, and
still does not support armv7-ve.

I also built an internal Buildroot toolchain using GCC 4.9, its possible
targets include armv7ve.

> > This is already the case. They are only passed if
> > > BR2_GCC_TARGET_{ARCH,CPU,TUNE} are defined. Some architectures (such as
> > > ARM) defines all three, but other architectures (such as x86) do not.
> > >
> > Now I see. In this case, maybe the simplest would be to just leave
> > BR2_GCC_TARGET_ARCH
> > undefined in arch/Config.in.arm if the user selected a proper CPU (and
> not
> > just some generic one).
>
> Not sure what you mean by "not just some generic one". In the
> menuconfig, what we ask the user is to define the specific ARM core
> being used. From that we can derive the -march and -mcpu values. But
> indeed, the -march value is quite useless.
>
>
I meant giving an option like "Generic ARM" or something, and then letting
the user to choose -mtune and -march values.
The documentation mentions this "-mcpu=generic-arch is also permissible,
and is equivalent to -march=arch -mtune=generic-arch.".

I would use this to specify BIG.little architecture for the Cortex-A15,
which in not available right now in the CPU section.
This requires a value for -mcpu=cortex-a15.cortex-a7 (also not available in
Linaro).

Kind regards,
Karoly
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20140514/19aa8cec/attachment.html>

[Buildroot] [PATCH 0/1] Fix redundant usage of -mcpu and -march/-mtune

[Károly Kasza]

Hi Thomas,

Looks like the armv7-a conflict is well known in GCC with Cortex A7 and
A15. These 2 CPU types won't accept -march=armv7-a, see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57907 . The solution is
armv7ve that you mentioned, but only intruduced in gcc 4.9 on the 29th
January (the external toolchains including Linaro's don't support this yet).

I think in Buildroot arch/Config.in.arm should be altered and
set BR2_GCC_TARGET_ARCH to none for the Cortex A7 and A15.

BR
Karoly



On Wed, May 14, 2014 at 9:59 AM, Thomas Petazzoni <
thomas.petazzoni@free-electrons.com> wrote:

> Dear K?roly Kasza,
>
> On Wed, 14 May 2014 09:55:09 +0200, K?roly Kasza wrote:
>
> > > So normally, we should set BR2_GCC_TARGET_ARCH to armv7ve for those
> > > Cortex-A variants. I'm currently experimenting with this, but I'm
> > > having a few issues. Also, it's likely that armv7ve is quite recent,
> > > and may not be present in older gcc versions.
> > >
> > >
> > That's the case with the Linaro 2014.02 toolchain - in GCC 4.8.2 the
> > armv7ve does not exist
> > (
> http://gcc.gnu.org/onlinedocs/gcc-4.8.2/gcc/ARM-Options.html#ARM-Options).
>
> Yeah, in gcc 4.8.2 vanilla. But Linaro toolchains don't use a vanilla
> gcc, they have lots of ARM related patches. And if the Linaro
> toolchains complains that -march=armv7-a is incompatible with
> -mcpu=cortex-a7, then quite certainly it means that cortex-a7 is part
> of a different -march family, according to this toolchain.
>
> > > This is already the case. They are only passed if
> > > BR2_GCC_TARGET_{ARCH,CPU,TUNE} are defined. Some architectures (such as
> > > ARM) defines all three, but other architectures (such as x86) do not.
> > >
> > Now I see. In this case, maybe the simplest would be to just leave
> > BR2_GCC_TARGET_ARCH
> > undefined in arch/Config.in.arm if the user selected a proper CPU (and
> not
> > just some generic one).
>
> Not sure what you mean by "not just some generic one". In the
> menuconfig, what we ask the user is to define the specific ARM core
> being used. From that we can derive the -march and -mcpu values. But
> indeed, the -march value is quite useless.
>
> However, I believe there are place in Buildroot where we do use the
> BR2_GCC_TARGET_ARCH value for other things than just the compiler
> flags. We would have to audit those before removing BR2_GCC_TARGET_ARCH
> from Config.in.arm.
>
> > Also, based on the documentation it should be completely unnecessary to
> > define -march if we have the accurate CPU with -mcpu (on ARM at least).
>
> Correct.
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20140527/1957d79c/attachment.html>