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

login
register
mail settings
Submitter Karoly Kasza
Date May 13, 2014, 1:58 p.m.
Message ID <1399989494-21343-2-git-send-email-kaszak@gmail.com>
Download mbox | patch
Permalink /patch/348364/
State Rejected
Headers show

Comments

Karoly Kasza - May 13, 2014, 1:58 p.m.
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(-)
Thomas Petazzoni - Nov. 1, 2014, 9:22 p.m.
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
Karoly Kasza - Nov. 1, 2014, 9:24 p.m.
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
>

Patch

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_)