[PATCH RFC] ARM: use -marm unconditionally for THUMB2_KERNEL=n builds

[PATCH RFC] ARM: use -marm unconditionally for THUMB2_KERNEL=n builds

[Uwe Kleine-König]

When using a toolchain that defaults to v7-m code generation using
cc-option fails to add -marm because it conflicts with the default cpu
type:

	$ echo > test.c
	$ arm-cortexm3-uclinuxeabi-gcc -marm -c test.c
	test.c:1:0: error: target CPU does not support ARM mode

resulting in errors like

	Error: selected processor does not support Thumb mode `mrs r1,cpsr'

Dropping the use of cc-option and using -marm unconditionally works fine
for this compiler because it's only ever used together with $(arch-y)
(e.g. -march=armv4).

The only possible culprit is a compiler that doesn't understand -marm.
My compiler collection only goes back to 4.0.3 which does work with this
option. The use of cc-option to test for -marm was introduced in commit
5636810d6f17 ([ARM] 3982/2: Explicitly select 32-bit ARM ISA (-marm))
back in 2006 when the minimal compiler version was already 3.3.

The next best fix is using

	CFLAGS_ISA := $(call cc-option,$(arch-y) -marm,)

and dropping arch-y from KBUILD_CFLAGS in case this change breaks gcc
3.x.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
Hello,

Arnd told me on irc that Nico did some build tests with ancient
compilers some time ago. Maybe you can tell which was the first compiler
to support -marm?

Best regards
Uwe

 arch/arm/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 0ce9d0f71f2a..6773c74a8f8b 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -115,8 +115,8 @@ ifeq ($(CONFIG_THUMB2_AVOID_R_ARM_THM_JUMP11),y)
 CFLAGS_MODULE	+=-fno-optimize-sibling-calls
 endif
 else
-CFLAGS_ISA	:=$(call cc-option,-marm,)
-AFLAGS_ISA	:=$(CFLAGS_ISA)
+CFLAGS_ISA	:= -marm
+AFLAGS_ISA	:= $(CFLAGS_ISA)
 endif
 
 # Need -Uarm for gcc < 3.x
-- 
2.1.4

[PATCH RFC] ARM: use -marm unconditionally for THUMB2_KERNEL=n builds

[Russell King - ARM Linux]

On Wed, May 27, 2015 at 11:42:07AM +0200, Uwe Kleine-K?nig wrote:
> The only possible culprit is a compiler that doesn't understand -marm.
> My compiler collection only goes back to 4.0.3 which does work with this
> option. The use of cc-option to test for -marm was introduced in commit
> 5636810d6f17 ([ARM] 3982/2: Explicitly select 32-bit ARM ISA (-marm))
> back in 2006 when the minimal compiler version was already 3.3.

According to gcc 3.3's --target-help, it supports -mthumb, but not -marm.

What we could possibly do is to change the := to a plain =, and then
evaluate all the options together via:

KBUILD_CFLAGS :=$(KBUILD_CFLAGS)

after their final +=.

In any case, please don't add a after the =, I've found that certain gnu
make flavours like to collect all that white space up into the executed
command, which makes reading the verbose make output more annoying.  Rule
number one of modification: stick to the established style in the file,
even if it's wrong.  Do style modifications separately and uniformly.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH RFC] ARM: use -marm unconditionally for THUMB2_KERNEL=n builds

[Uwe Kleine-König]

Hello Russell,

On Wed, May 27, 2015 at 10:53:39AM +0100, Russell King - ARM Linux wrote:
> On Wed, May 27, 2015 at 11:42:07AM +0200, Uwe Kleine-K?nig wrote:
> > The only possible culprit is a compiler that doesn't understand -marm.
> > My compiler collection only goes back to 4.0.3 which does work with this
> > option. The use of cc-option to test for -marm was introduced in commit
> > 5636810d6f17 ([ARM] 3982/2: Explicitly select 32-bit ARM ISA (-marm))
> > back in 2006 when the minimal compiler version was already 3.3.
> 
> According to gcc 3.3's --target-help, it supports -mthumb, but not -marm.
> 
> What we could possibly do is to change the := to a plain =, and then
> evaluate all the options together via:
> 
> KBUILD_CFLAGS :=$(KBUILD_CFLAGS)
You mean

	KBUILD_CFLAGS += $(arch-y)

as soon as arch-y is defined to assert it's already present when -marm
is tested for, right? I will give that a try.

> after their final +=.
> 
> In any case, please don't add a after the =, I've found that certain gnu
> make flavours like to collect all that white space up into the executed
> command, which makes reading the verbose make output more annoying.  Rule
> number one of modification: stick to the established style in the file,
> even if it's wrong.  Do style modifications separately and uniformly.
ok

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH RFC] ARM: use -marm unconditionally for THUMB2_KERNEL=n builds

[Nicolas Pitre]

On Wed, 27 May 2015, Uwe Kleine-K?nig wrote:

> Arnd told me on irc that Nico did some build tests with ancient
> compilers some time ago. Maybe you can tell which was the first compiler
> to support -marm?

I didn't go beyond gcc 4.0.2. This is not going to help you much.


Nicolas

[PATCH RFC] ARM: use -marm unconditionally for THUMB2_KERNEL=n builds

[Russell King - ARM Linux]

On Wed, May 27, 2015 at 04:59:21PM +0200, Uwe Kleine-K?nig wrote:
> Hello Russell,
> 
> On Wed, May 27, 2015 at 10:53:39AM +0100, Russell King - ARM Linux wrote:
> > On Wed, May 27, 2015 at 11:42:07AM +0200, Uwe Kleine-K?nig wrote:
> > > The only possible culprit is a compiler that doesn't understand -marm.
> > > My compiler collection only goes back to 4.0.3 which does work with this
> > > option. The use of cc-option to test for -marm was introduced in commit
> > > 5636810d6f17 ([ARM] 3982/2: Explicitly select 32-bit ARM ISA (-marm))
> > > back in 2006 when the minimal compiler version was already 3.3.
> > 
> > According to gcc 3.3's --target-help, it supports -mthumb, but not -marm.
> > 
> > What we could possibly do is to change the := to a plain =, and then
> > evaluate all the options together via:
> > 
> > KBUILD_CFLAGS :=$(KBUILD_CFLAGS)
> You mean
> 
> 	KBUILD_CFLAGS += $(arch-y)
> 
> as soon as arch-y is defined to assert it's already present when -marm
> is tested for, right? I will give that a try.

I mean:

 else
-CFLAGS_ISA	:=$(call cc-option,-marm,)
-AFLAGS_ISA	:=$(CFLAGS_ISA)
+CFLAGS_ISA	=$(call cc-option,-marm,)
+AFLAGS_ISA	=$(CFLAGS_ISA)
 endif
...
-KBUILD_CFLAGS	+=$(CFLAGS_ABI) $(CFLAGS_ISA) $(arch-y) $(tune-y) $(call cc-option,-mshort-load-bytes,$(call cc-option,-malignment-traps,)) -msoft-float -Uarm
-KBUILD_AFLAGS	+=$(CFLAGS_ABI) $(AFLAGS_ISA) $(arch-y) $(tune-y) -include asm/unified.h -msoft-float
+CFLAGS_TRAPS	:=$(call cc-option,-mshort-load-bytes,$(call cc-option,-malignment-traps,))
+
+KBUILD_CFLAGS	+=$(CFLAGS_ABI) $(arch-y) $(tune-y) $(CFLAGS_TRAPS) -msoft-float -Uarm
+KBUILD_AFLAGS	+=$(CFLAGS_ABI) $(arch-y) $(tune-y) -include asm/unified.h -msoft-float

+# Ensures that all previous $(call ...) options are evaluated once
+KBUILD_CFLAGS	:=$(CFLAGS_ISA) $(KBUILD_CFLAGS)
+KBUILD_AFLAGS	:=$(CFLAGS_ISA) $(KBUILD_AFLAGS)

This has the effect that "-marm" will be evaluated with all the other
KBUILD_CFLAGS options already set.  (I think I have this right...)

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH RFC] ARM: use -marm unconditionally for THUMB2_KERNEL=n builds

[Dave Martin]

On Wed, May 27, 2015 at 11:42:07AM +0200, Uwe Kleine-K?nig wrote:
> When using a toolchain that defaults to v7-m code generation using
> cc-option fails to add -marm because it conflicts with the default cpu
> type:
> 
> 	$ echo > test.c
> 	$ arm-cortexm3-uclinuxeabi-gcc -marm -c test.c
> 	test.c:1:0: error: target CPU does not support ARM mode
> 
> resulting in errors like
> 
> 	Error: selected processor does not support Thumb mode `mrs r1,cpsr'
> 
> Dropping the use of cc-option and using -marm unconditionally works fine
> for this compiler because it's only ever used together with $(arch-y)
> (e.g. -march=armv4).

cc-option passes the compiler flags already selected via KBUILD_CFLAGS,
so hoisting some of the append to KBUILD_CFLAGS earlier would probably
help:

KBUILD_CFLAGS += $(CFLAGS_ABI) $(arch-y) $(tune-y)

...

CFLAGS_ISA := $(call cc-option,-marm,)

...

KBUILD_CFLAGS += $(CFLAGS_ISA)


That should remove any risk of toolchain incompatibility -- but I've
not tested it...

Cheers
---Dave

[...]

[PATCH RFC] ARM: use -marm unconditionally for THUMB2_KERNEL=n builds

[Dave Martin]

On Wed, May 27, 2015 at 06:46:57PM +0100, Russell King - ARM Linux wrote:
> On Wed, May 27, 2015 at 04:59:21PM +0200, Uwe Kleine-K?nig wrote:
> > Hello Russell,
> > 
> > On Wed, May 27, 2015 at 10:53:39AM +0100, Russell King - ARM Linux wrote:
> > > On Wed, May 27, 2015 at 11:42:07AM +0200, Uwe Kleine-K?nig wrote:
> > > > The only possible culprit is a compiler that doesn't understand -marm.
> > > > My compiler collection only goes back to 4.0.3 which does work with this
> > > > option. The use of cc-option to test for -marm was introduced in commit
> > > > 5636810d6f17 ([ARM] 3982/2: Explicitly select 32-bit ARM ISA (-marm))
> > > > back in 2006 when the minimal compiler version was already 3.3.
> > > 
> > > According to gcc 3.3's --target-help, it supports -mthumb, but not -marm.
> > > 
> > > What we could possibly do is to change the := to a plain =, and then
> > > evaluate all the options together via:
> > > 
> > > KBUILD_CFLAGS :=$(KBUILD_CFLAGS)
> > You mean
> > 
> > 	KBUILD_CFLAGS += $(arch-y)
> > 
> > as soon as arch-y is defined to assert it's already present when -marm
> > is tested for, right? I will give that a try.
> 
> I mean:
> 
>  else
> -CFLAGS_ISA	:=$(call cc-option,-marm,)
> -AFLAGS_ISA	:=$(CFLAGS_ISA)
> +CFLAGS_ISA	=$(call cc-option,-marm,)
> +AFLAGS_ISA	=$(CFLAGS_ISA)
>  endif
> ...
> -KBUILD_CFLAGS	+=$(CFLAGS_ABI) $(CFLAGS_ISA) $(arch-y) $(tune-y) $(call cc-option,-mshort-load-bytes,$(call cc-option,-malignment-traps,)) -msoft-float -Uarm
> -KBUILD_AFLAGS	+=$(CFLAGS_ABI) $(AFLAGS_ISA) $(arch-y) $(tune-y) -include asm/unified.h -msoft-float
> +CFLAGS_TRAPS	:=$(call cc-option,-mshort-load-bytes,$(call cc-option,-malignment-traps,))
> +
> +KBUILD_CFLAGS	+=$(CFLAGS_ABI) $(arch-y) $(tune-y) $(CFLAGS_TRAPS) -msoft-float -Uarm
> +KBUILD_AFLAGS	+=$(CFLAGS_ABI) $(arch-y) $(tune-y) -include asm/unified.h -msoft-float
> 
> +# Ensures that all previous $(call ...) options are evaluated once
> +KBUILD_CFLAGS	:=$(CFLAGS_ISA) $(KBUILD_CFLAGS)
> +KBUILD_AFLAGS	:=$(CFLAGS_ISA) $(KBUILD_AFLAGS)
> 
> This has the effect that "-marm" will be evaluated with all the other
> KBUILD_CFLAGS options already set.  (I think I have this right...)

That should work too.  It requires less code rearrangement than my
approach.

[...]

Cheers
---Dave