| Message ID | 1315411158-17479-2-git-send-email-dave.martin@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 7 Sep 2011, Eric Miao wrote: > On Wed, Sep 7, 2011 at 8:59 AM, Dave Martin <dave.martin@linaro.org> wrote: > > Because gcc/gas have no sane way to turn on individual CPU > > extensions from the command-line, iwmmxt.S was previously built > > with -mcpu=iwmmxt. Unfortunately, this also downgrades the CPU to > > v5, with the result that this file fails to build for a Thumb-2 > > kernel. > > > > New versions of the tools support -march=<base arch>+iwmmxt, and it > > seems reasonable to require up-to-date tools when building in > > Thumb-2. So, this patch uses -march=armv7-a+iwmmxt for > > CONFIG_THUMB2_KERNEL=y. > > > > Signed-off-by: Dave Martin <dave.martin@linaro.org> > > --- > > arch/arm/kernel/Makefile | 7 +++++++ > > 1 files changed, 7 insertions(+), 0 deletions(-) > > > > diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile > > index f7887dc..8a58339 100644 > > --- a/arch/arm/kernel/Makefile > > +++ b/arch/arm/kernel/Makefile > > @@ -65,7 +65,14 @@ obj-$(CONFIG_CPU_PJ4) += pj4-cp0.o > > obj-$(CONFIG_IWMMXT) += iwmmxt.o > > obj-$(CONFIG_CPU_HAS_PMU) += pmu.o > > obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o > > + > > +# When enough people have binutils which support -march=...+iwmmxt, this > > +# should change to something like if __LINUX_ARM_ARCH__ < 7. > > +ifdef CONFIG_THUMB2_KERNEL > > +AFLAGS_iwmmxt.o := -Wa,-march=armv7-a+iwmmxt > > +else > > AFLAGS_iwmmxt.o := -Wa,-mcpu=iwmmxt > > +endif > > It looks more like the switch should depend on the compiler version. > Unless there is a clear way to decide if gcc supports this switch, I > think it's reasonable to have the change like above. Normally the way to go with gcc version dependent alternatives is to use something like: AFLAGS_foo.o := $(call cc-option,<the_new_flag>,<the_fallback_flag>) This will test if <the_new_flag> is supported by the used gcc, and use the fallback otherwise. Nicolas
On Wednesday 07 September 2011 13:18:21 Nicolas Pitre wrote: > > > + > > > +# When enough people have binutils which support -march=...+iwmmxt, this > > > +# should change to something like if __LINUX_ARM_ARCH__ < 7. > > > +ifdef CONFIG_THUMB2_KERNEL > > > +AFLAGS_iwmmxt.o := -Wa,-march=armv7-a+iwmmxt > > > +else > > > AFLAGS_iwmmxt.o := -Wa,-mcpu=iwmmxt > > > +endif > > > > It looks more like the switch should depend on the compiler version. > > Unless there is a clear way to decide if gcc supports this switch, I > > think it's reasonable to have the change like above. > > Normally the way to go with gcc version dependent alternatives is to use > something like: > > AFLAGS_foo.o := $(call cc-option,<the_new_flag>,<the_fallback_flag>) > > This will test if <the_new_flag> is supported by the used gcc, and use > the fallback otherwise. Yes, that's possible here, but it's not actually correct either, because the CPU core that we are running on is either a v5 XScale with iwmmxt or a v7 pj4 with iwmmxt. Now, it should not really matter if we build the code with flags for a different more complex instruction set, but it can potentially hide bugs. I think the simple solution that Dave posted is actually more appropriate. The three possible cases are: v5+iwmmxt: always use -Wa,-mcpu=iwmmxt as we've always done, and it's correct v7+iwmmxt+arm: still use -Wa,-mcpu=iwmmxt, not correct but close enough and is known to build the file with all existing toolchaings v7+iwmmxt+thumb2: always use -Wa,-march=armv7-a+iwmmxt, which is correct and the only possible way to build this file anyway. Old toolchains will fail and there is nothing we can do about it. Arnd
On Wed, Sep 07, 2011 at 10:32:09PM +0200, Arnd Bergmann wrote: > On Wednesday 07 September 2011 13:18:21 Nicolas Pitre wrote: > > > > + > > > > +# When enough people have binutils which support -march=...+iwmmxt, this > > > > +# should change to something like if __LINUX_ARM_ARCH__ < 7. > > > > +ifdef CONFIG_THUMB2_KERNEL > > > > +AFLAGS_iwmmxt.o := -Wa,-march=armv7-a+iwmmxt > > > > +else > > > > AFLAGS_iwmmxt.o := -Wa,-mcpu=iwmmxt > > > > +endif > > > > > > It looks more like the switch should depend on the compiler version. > > > Unless there is a clear way to decide if gcc supports this switch, I > > > think it's reasonable to have the change like above. > > > > Normally the way to go with gcc version dependent alternatives is to use > > something like: > > > > AFLAGS_foo.o := $(call cc-option,<the_new_flag>,<the_fallback_flag>) > > > > This will test if <the_new_flag> is supported by the used gcc, and use > > the fallback otherwise. > > Yes, that's possible here, but it's not actually correct either, because the > CPU core that we are running on is either a v5 XScale with iwmmxt or > a v7 pj4 with iwmmxt. Now, it should not really matter if we build the > code with flags for a different more complex instruction set, but it can > potentially hide bugs. > > I think the simple solution that Dave posted is actually more appropriate. > The three possible cases are: > > v5+iwmmxt: always use -Wa,-mcpu=iwmmxt as we've always done, and it's correct > v7+iwmmxt+arm: still use -Wa,-mcpu=iwmmxt, not correct but close enough and > is known to build the file with all existing toolchaings > v7+iwmmxt+thumb2: always use -Wa,-march=armv7-a+iwmmxt, which is correct and > the only possible way to build this file anyway. Old toolchains > will fail and there is nothing we can do about it. There is another option, which is to use cc-option and then check the result in AFLAGS_iwmmxt.o, throwing an error from the Makefile as necessary. I'll have a go at that if nobody has any objections. There doesn't seem to be any cleaner way to catch this error -- compiler version issues are invisible to Kconfig. Cheers ---Dave
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile index f7887dc..8a58339 100644 --- a/arch/arm/kernel/Makefile +++ b/arch/arm/kernel/Makefile @@ -65,7 +65,14 @@ obj-$(CONFIG_CPU_PJ4) += pj4-cp0.o obj-$(CONFIG_IWMMXT) += iwmmxt.o obj-$(CONFIG_CPU_HAS_PMU) += pmu.o obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o + +# When enough people have binutils which support -march=...+iwmmxt, this +# should change to something like if __LINUX_ARM_ARCH__ < 7. +ifdef CONFIG_THUMB2_KERNEL +AFLAGS_iwmmxt.o := -Wa,-march=armv7-a+iwmmxt +else AFLAGS_iwmmxt.o := -Wa,-mcpu=iwmmxt +endif ifneq ($(CONFIG_ARCH_EBSA110),y) obj-y += io.o
Because gcc/gas have no sane way to turn on individual CPU extensions from the command-line, iwmmxt.S was previously built with -mcpu=iwmmxt. Unfortunately, this also downgrades the CPU to v5, with the result that this file fails to build for a Thumb-2 kernel. New versions of the tools support -march=<base arch>+iwmmxt, and it seems reasonable to require up-to-date tools when building in Thumb-2. So, this patch uses -march=armv7-a+iwmmxt for CONFIG_THUMB2_KERNEL=y. Signed-off-by: Dave Martin <dave.martin@linaro.org> --- arch/arm/kernel/Makefile | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)