diff mbox

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

Message ID 1432719727-28860-1-git-send-email-u.kleine-koenig@pengutronix.de
State New
Headers show

Commit Message

Uwe Kleine-König May 27, 2015, 9:42 a.m. UTC
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(-)

Comments

Russell King - ARM Linux May 27, 2015, 9:53 a.m. UTC | #1
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.
Uwe Kleine-König May 27, 2015, 2:59 p.m. UTC | #2
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
Nicolas Pitre May 27, 2015, 4:25 p.m. UTC | #3
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
Russell King - ARM Linux May 27, 2015, 5:46 p.m. UTC | #4
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...)
diff mbox

Patch

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