diff mbox

[U-Boot,v2,3/5] ARM: enable Thumb build

Message ID 1330004385-22502-3-git-send-email-aneesh@ti.com
State Superseded
Headers show

Commit Message

Aneesh V Feb. 23, 2012, 1:39 p.m. UTC
Enable Thumb build and ARM-Thumb interworking based on the new
config flag CONFIG_SYS_THUMB_BUILD

Signed-off-by: Aneesh V <aneesh@ti.com>
---
Changes from RFC to V1:
- Fixed review comments from Tom Rini <trini@ti.com>

Changes from V1 to V2:
- None
---
 README             |    8 ++++++++
 arch/arm/config.mk |   20 ++++++++++++++------
 2 files changed, 22 insertions(+), 6 deletions(-)

Comments

Mike Frysinger Feb. 23, 2012, 2:57 p.m. UTC | #1
On Thursday 23 February 2012 08:39:43 Aneesh V wrote:
> --- a/arch/arm/config.mk
> +++ b/arch/arm/config.mk
> 
> -# Explicitly specifiy 32-bit ARM ISA since toolchain default can be
> -mthumb: +# Choose between ARM/Thumb instruction sets
> +ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
> +PF_CPPFLAGS_ARM := $(call cc-option, -mthumb -mthumb-interwork,\
> +			$(call cc-option,-marm,)\
> +			$(call cc-option,-mno-thumb-interwork,)\
> +		)
> +else
>  PF_CPPFLAGS_ARM := $(call cc-option,-marm,)
> +PF_CPPFLAGS_ARM += $(call cc-option,-mno-thumb-interwork,)

this 2nd part is no good.  "+=" is not the same thing as ":=".

might be simpler to do:
PF_CPPFLAGS_MARM := $(call cc-option,-marm)
PF_CPPFLAGS_THUMB := $(call cc-option,-mthumb -mthumb-interwork)
PF_CPPFLAGS_NO_THUMB := $(call cc-option,-mno-thumb-interwork)

ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
PF_CPPFLAGS_ARM = $(PF_CPPFLAGS_THUMB)
else
PF_CPPFLAGS_ARM = $(PF_CPPFLAGS_MARM) $(PF_CPPFLAGS_NO_THUMB)
endif
-mike
Aneesh V Feb. 23, 2012, 5:28 p.m. UTC | #2
On Thursday 23 February 2012 08:27 PM, Mike Frysinger wrote:
> On Thursday 23 February 2012 08:39:43 Aneesh V wrote:
>> --- a/arch/arm/config.mk
>> +++ b/arch/arm/config.mk
>>
>> -# Explicitly specifiy 32-bit ARM ISA since toolchain default can be
>> -mthumb: +# Choose between ARM/Thumb instruction sets
>> +ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
>> +PF_CPPFLAGS_ARM := $(call cc-option, -mthumb -mthumb-interwork,\
>> +			$(call cc-option,-marm,)\
>> +			$(call cc-option,-mno-thumb-interwork,)\
>> +		)
>> +else
>>   PF_CPPFLAGS_ARM := $(call cc-option,-marm,)
>> +PF_CPPFLAGS_ARM += $(call cc-option,-mno-thumb-interwork,)
>
> this 2nd part is no good.  "+=" is not the same thing as ":=".

I don't understand the difference. '+=' is done after ':=' right?

>
> might be simpler to do:
> PF_CPPFLAGS_MARM := $(call cc-option,-marm)
> PF_CPPFLAGS_THUMB := $(call cc-option,-mthumb -mthumb-interwork)
> PF_CPPFLAGS_NO_THUMB := $(call cc-option,-mno-thumb-interwork)
>
> ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
> PF_CPPFLAGS_ARM = $(PF_CPPFLAGS_THUMB)
> else
> PF_CPPFLAGS_ARM = $(PF_CPPFLAGS_MARM) $(PF_CPPFLAGS_NO_THUMB)
> endif
> -mike

Are you trying to avoid all '+='. If so, why?
Tom Rini Feb. 23, 2012, 5:34 p.m. UTC | #3
On Thu, Feb 23, 2012 at 10:58:36PM +0530, Aneesh V wrote:
> On Thursday 23 February 2012 08:27 PM, Mike Frysinger wrote:
> >On Thursday 23 February 2012 08:39:43 Aneesh V wrote:
> >>--- a/arch/arm/config.mk
> >>+++ b/arch/arm/config.mk
> >>
> >>-# Explicitly specifiy 32-bit ARM ISA since toolchain default can be
> >>-mthumb: +# Choose between ARM/Thumb instruction sets
> >>+ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
> >>+PF_CPPFLAGS_ARM := $(call cc-option, -mthumb -mthumb-interwork,\
> >>+			$(call cc-option,-marm,)\
> >>+			$(call cc-option,-mno-thumb-interwork,)\
> >>+		)
> >>+else
> >>  PF_CPPFLAGS_ARM := $(call cc-option,-marm,)
> >>+PF_CPPFLAGS_ARM += $(call cc-option,-mno-thumb-interwork,)
> >
> >this 2nd part is no good.  "+=" is not the same thing as ":=".
> 
> I don't understand the difference. '+=' is done after ':=' right?

'+=' is evaluated every file we build, ':=' is evaluated once.  We use
the latter to keep build times down.
Aneesh V Feb. 23, 2012, 5:49 p.m. UTC | #4
On Thursday 23 February 2012 11:04 PM, Tom Rini wrote:
> On Thu, Feb 23, 2012 at 10:58:36PM +0530, Aneesh V wrote:
>> On Thursday 23 February 2012 08:27 PM, Mike Frysinger wrote:
>>> On Thursday 23 February 2012 08:39:43 Aneesh V wrote:
>>>> --- a/arch/arm/config.mk
>>>> +++ b/arch/arm/config.mk
>>>>
>>>> -# Explicitly specifiy 32-bit ARM ISA since toolchain default can be
>>>> -mthumb: +# Choose between ARM/Thumb instruction sets
>>>> +ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
>>>> +PF_CPPFLAGS_ARM := $(call cc-option, -mthumb -mthumb-interwork,\
>>>> +			$(call cc-option,-marm,)\
>>>> +			$(call cc-option,-mno-thumb-interwork,)\
>>>> +		)
>>>> +else
>>>>   PF_CPPFLAGS_ARM := $(call cc-option,-marm,)
>>>> +PF_CPPFLAGS_ARM += $(call cc-option,-mno-thumb-interwork,)
>>>
>>> this 2nd part is no good.  "+=" is not the same thing as ":=".
>>
>> I don't understand the difference. '+=' is done after ':=' right?
>
> '+=' is evaluated every file we build, ':=' is evaluated once.  We use
> the latter to keep build times down.
>

Ok. so, are we trying to reduce the number of "+=", right?

Thanks for clarifying.

br,
Aneesh
Tom Rini Feb. 23, 2012, 5:51 p.m. UTC | #5
On Thu, Feb 23, 2012 at 11:19:59PM +0530, Aneesh V wrote:
> On Thursday 23 February 2012 11:04 PM, Tom Rini wrote:
> >On Thu, Feb 23, 2012 at 10:58:36PM +0530, Aneesh V wrote:
> >>On Thursday 23 February 2012 08:27 PM, Mike Frysinger wrote:
> >>>On Thursday 23 February 2012 08:39:43 Aneesh V wrote:
> >>>>--- a/arch/arm/config.mk
> >>>>+++ b/arch/arm/config.mk
> >>>>
> >>>>-# Explicitly specifiy 32-bit ARM ISA since toolchain default can be
> >>>>-mthumb: +# Choose between ARM/Thumb instruction sets
> >>>>+ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
> >>>>+PF_CPPFLAGS_ARM := $(call cc-option, -mthumb -mthumb-interwork,\
> >>>>+			$(call cc-option,-marm,)\
> >>>>+			$(call cc-option,-mno-thumb-interwork,)\
> >>>>+		)
> >>>>+else
> >>>>  PF_CPPFLAGS_ARM := $(call cc-option,-marm,)
> >>>>+PF_CPPFLAGS_ARM += $(call cc-option,-mno-thumb-interwork,)
> >>>
> >>>this 2nd part is no good.  "+=" is not the same thing as ":=".
> >>
> >>I don't understand the difference. '+=' is done after ':=' right?
> >
> >'+=' is evaluated every file we build, ':=' is evaluated once.  We use
> >the latter to keep build times down.
> >
> 
> Ok. so, are we trying to reduce the number of "+=", right?

Yes, it should already be at or near 0 uses.
Mike Frysinger Feb. 23, 2012, 6:04 p.m. UTC | #6
On Thursday 23 February 2012 12:28:36 Aneesh V wrote:
> On Thursday 23 February 2012 08:27 PM, Mike Frysinger wrote:
> > On Thursday 23 February 2012 08:39:43 Aneesh V wrote:
> >> --- a/arch/arm/config.mk
> >> +++ b/arch/arm/config.mk
> >> 
> >> -# Explicitly specifiy 32-bit ARM ISA since toolchain default can be
> >> -mthumb: +# Choose between ARM/Thumb instruction sets
> >> +ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
> >> +PF_CPPFLAGS_ARM := $(call cc-option, -mthumb -mthumb-interwork,\
> >> +			$(call cc-option,-marm,)\
> >> +			$(call cc-option,-mno-thumb-interwork,)\
> >> +		)
> >> +else
> >> 
> >>   PF_CPPFLAGS_ARM := $(call cc-option,-marm,)
> >> 
> >> +PF_CPPFLAGS_ARM += $(call cc-option,-mno-thumb-interwork,)
> > 
> > this 2nd part is no good.  "+=" is not the same thing as ":=".
> 
> I don't understand the difference. '+=' is done after ':=' right?
> 
> > might be simpler to do:
> > PF_CPPFLAGS_MARM := $(call cc-option,-marm)
> > PF_CPPFLAGS_THUMB := $(call cc-option,-mthumb -mthumb-interwork)
> > PF_CPPFLAGS_NO_THUMB := $(call cc-option,-mno-thumb-interwork)
> > 
> > ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
> > PF_CPPFLAGS_ARM = $(PF_CPPFLAGS_THUMB)
> > else
> > PF_CPPFLAGS_ARM = $(PF_CPPFLAGS_MARM) $(PF_CPPFLAGS_NO_THUMB)
> > endif
> 
> Are you trying to avoid all '+='. If so, why?

"+=" does delayed evaluation and is the whole reason we started using ":=" in 
makefiles for *computed* values

when you do:
	FOO := $(call cc-option,-marm)
you're storing the result of the computation in $(FOO)

if you do:
	FOO += $(call cc-option,-marm)
you're appending "$(call cc-option,-marm)" to $(FOO) and that won't actually 
get computed until $(FOO) gets used

so if you append $(call ...) to $(CPPFLAGS), then you end up doing the cc-
option computation every time you compile a file that uses $(CPPFLAGS)
-mike
Mike Frysinger Feb. 23, 2012, 6:05 p.m. UTC | #7
On Thursday 23 February 2012 12:49:59 Aneesh V wrote:
> On Thursday 23 February 2012 11:04 PM, Tom Rini wrote:
> > On Thu, Feb 23, 2012 at 10:58:36PM +0530, Aneesh V wrote:
> >> On Thursday 23 February 2012 08:27 PM, Mike Frysinger wrote:
> >>> On Thursday 23 February 2012 08:39:43 Aneesh V wrote:
> >>>> --- a/arch/arm/config.mk
> >>>> +++ b/arch/arm/config.mk
> >>>> 
> >>>> -# Explicitly specifiy 32-bit ARM ISA since toolchain default can be
> >>>> -mthumb: +# Choose between ARM/Thumb instruction sets
> >>>> +ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
> >>>> +PF_CPPFLAGS_ARM := $(call cc-option, -mthumb -mthumb-interwork,\
> >>>> +			$(call cc-option,-marm,)\
> >>>> +			$(call cc-option,-mno-thumb-interwork,)\
> >>>> +		)
> >>>> +else
> >>>> 
> >>>>   PF_CPPFLAGS_ARM := $(call cc-option,-marm,)
> >>>> 
> >>>> +PF_CPPFLAGS_ARM += $(call cc-option,-mno-thumb-interwork,)
> >>> 
> >>> this 2nd part is no good.  "+=" is not the same thing as ":=".
> >> 
> >> I don't understand the difference. '+=' is done after ':=' right?
> > 
> > '+=' is evaluated every file we build, ':=' is evaluated once.  We use
> > the latter to keep build times down.
> 
> Ok. so, are we trying to reduce the number of "+=", right?

for things that require computation, yes.  if it's just string values or other 
variables, then it doesn't matter and avoiding ":=" is preferred.
-mike
Aneesh V Feb. 23, 2012, 6:09 p.m. UTC | #8
On Thursday 23 February 2012 11:21 PM, Tom Rini wrote:
> On Thu, Feb 23, 2012 at 11:19:59PM +0530, Aneesh V wrote:
>> On Thursday 23 February 2012 11:04 PM, Tom Rini wrote:
>>> On Thu, Feb 23, 2012 at 10:58:36PM +0530, Aneesh V wrote:
>>>> On Thursday 23 February 2012 08:27 PM, Mike Frysinger wrote:
>>>>> On Thursday 23 February 2012 08:39:43 Aneesh V wrote:
>>>>>> --- a/arch/arm/config.mk
>>>>>> +++ b/arch/arm/config.mk
>>>>>>
>>>>>> -# Explicitly specifiy 32-bit ARM ISA since toolchain default can be
>>>>>> -mthumb: +# Choose between ARM/Thumb instruction sets
>>>>>> +ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
>>>>>> +PF_CPPFLAGS_ARM := $(call cc-option, -mthumb -mthumb-interwork,\
>>>>>> +			$(call cc-option,-marm,)\
>>>>>> +			$(call cc-option,-mno-thumb-interwork,)\
>>>>>> +		)
>>>>>> +else
>>>>>>   PF_CPPFLAGS_ARM := $(call cc-option,-marm,)
>>>>>> +PF_CPPFLAGS_ARM += $(call cc-option,-mno-thumb-interwork,)
>>>>>
>>>>> this 2nd part is no good.  "+=" is not the same thing as ":=".
>>>>
>>>> I don't understand the difference. '+=' is done after ':=' right?
>>>
>>> '+=' is evaluated every file we build, ':=' is evaluated once.  We use
>>> the latter to keep build times down.
>>>
>>
>> Ok. so, are we trying to reduce the number of "+=", right?
>
> Yes, it should already be at or near 0 uses.

We need at least one for finally appending to the exported variable,
right. So, looks like one += for adding '-mthumb -mthumb-interwork'
together is better than having one each for the two options? Is that
the logic?
Aneesh V Feb. 23, 2012, 6:12 p.m. UTC | #9
On Thursday 23 February 2012 11:34 PM, Mike Frysinger wrote:
> On Thursday 23 February 2012 12:28:36 Aneesh V wrote:
>> On Thursday 23 February 2012 08:27 PM, Mike Frysinger wrote:
>>> On Thursday 23 February 2012 08:39:43 Aneesh V wrote:
>>>> --- a/arch/arm/config.mk
>>>> +++ b/arch/arm/config.mk
>>>>
>>>> -# Explicitly specifiy 32-bit ARM ISA since toolchain default can be
>>>> -mthumb: +# Choose between ARM/Thumb instruction sets
>>>> +ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
>>>> +PF_CPPFLAGS_ARM := $(call cc-option, -mthumb -mthumb-interwork,\
>>>> +			$(call cc-option,-marm,)\
>>>> +			$(call cc-option,-mno-thumb-interwork,)\
>>>> +		)
>>>> +else
>>>>
>>>>    PF_CPPFLAGS_ARM := $(call cc-option,-marm,)
>>>>
>>>> +PF_CPPFLAGS_ARM += $(call cc-option,-mno-thumb-interwork,)
>>>
>>> this 2nd part is no good.  "+=" is not the same thing as ":=".
>>
>> I don't understand the difference. '+=' is done after ':=' right?
>>
>>> might be simpler to do:
>>> PF_CPPFLAGS_MARM := $(call cc-option,-marm)
>>> PF_CPPFLAGS_THUMB := $(call cc-option,-mthumb -mthumb-interwork)
>>> PF_CPPFLAGS_NO_THUMB := $(call cc-option,-mno-thumb-interwork)
>>>
>>> ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
>>> PF_CPPFLAGS_ARM = $(PF_CPPFLAGS_THUMB)
>>> else
>>> PF_CPPFLAGS_ARM = $(PF_CPPFLAGS_MARM) $(PF_CPPFLAGS_NO_THUMB)
>>> endif
>>
>> Are you trying to avoid all '+='. If so, why?
>
> "+=" does delayed evaluation and is the whole reason we started using ":=" in
> makefiles for *computed* values
>
> when you do:
> 	FOO := $(call cc-option,-marm)
> you're storing the result of the computation in $(FOO)
>
> if you do:
> 	FOO += $(call cc-option,-marm)
> you're appending "$(call cc-option,-marm)" to $(FOO) and that won't actually
> get computed until $(FOO) gets used
>
> so if you append $(call ...) to $(CPPFLAGS), then you end up doing the cc-
> option computation every time you compile a file that uses $(CPPFLAGS)

Get it. Thanks for explaining. I missed the computation part.

br,
Aneesh


> -mike
Aneesh V Feb. 23, 2012, 6:13 p.m. UTC | #10
On Thursday 23 February 2012 11:39 PM, Aneesh V wrote:
> On Thursday 23 February 2012 11:21 PM, Tom Rini wrote:
>> On Thu, Feb 23, 2012 at 11:19:59PM +0530, Aneesh V wrote:
>>> On Thursday 23 February 2012 11:04 PM, Tom Rini wrote:
>>>> On Thu, Feb 23, 2012 at 10:58:36PM +0530, Aneesh V wrote:
>>>>> On Thursday 23 February 2012 08:27 PM, Mike Frysinger wrote:
>>>>>> On Thursday 23 February 2012 08:39:43 Aneesh V wrote:
>>>>>>> --- a/arch/arm/config.mk
>>>>>>> +++ b/arch/arm/config.mk
>>>>>>>
>>>>>>> -# Explicitly specifiy 32-bit ARM ISA since toolchain default can be
>>>>>>> -mthumb: +# Choose between ARM/Thumb instruction sets
>>>>>>> +ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
>>>>>>> +PF_CPPFLAGS_ARM := $(call cc-option, -mthumb -mthumb-interwork,\
>>>>>>> + $(call cc-option,-marm,)\
>>>>>>> + $(call cc-option,-mno-thumb-interwork,)\
>>>>>>> + )
>>>>>>> +else
>>>>>>> PF_CPPFLAGS_ARM := $(call cc-option,-marm,)
>>>>>>> +PF_CPPFLAGS_ARM += $(call cc-option,-mno-thumb-interwork,)
>>>>>>
>>>>>> this 2nd part is no good. "+=" is not the same thing as ":=".
>>>>>
>>>>> I don't understand the difference. '+=' is done after ':=' right?
>>>>
>>>> '+=' is evaluated every file we build, ':=' is evaluated once. We use
>>>> the latter to keep build times down.
>>>>
>>>
>>> Ok. so, are we trying to reduce the number of "+=", right?
>>
>> Yes, it should already be at or near 0 uses.
>
> We need at least one for finally appending to the exported variable,
> right. So, looks like one += for adding '-mthumb -mthumb-interwork'
> together is better than having one each for the two options? Is that
> the logic?
>

Please ignore this question. It's clear to me now with Mike's latest 
explanation. Thanks.
diff mbox

Patch

diff --git a/README b/README
index eba6378..1f4e2e8 100644
--- a/README
+++ b/README
@@ -426,6 +426,14 @@  The following options need to be configured:
 		Select high exception vectors of the ARM core, e.g., do not
 		clear the V bit of the c1 register of CP15.
 
+		CONFIG_SYS_THUMB_BUILD
+
+		Use this flag to build U-Boot using the Thumb instruction
+		set for ARM architectures. Thumb instruction set provides
+		better code density. For ARM architectures that support
+		Thumb2 this flag will result in Thumb2 code generated by
+		GCC.
+
 - Linux Kernel Interface:
 		CONFIG_CLOCKS_IN_MHZ
 
diff --git a/arch/arm/config.mk b/arch/arm/config.mk
index 45f9dca..de9aa53 100644
--- a/arch/arm/config.mk
+++ b/arch/arm/config.mk
@@ -33,25 +33,33 @@  endif
 
 PLATFORM_CPPFLAGS += -DCONFIG_ARM -D__ARM__
 
-# Explicitly specifiy 32-bit ARM ISA since toolchain default can be -mthumb:
+# Choose between ARM/Thumb instruction sets
+ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
+PF_CPPFLAGS_ARM := $(call cc-option, -mthumb -mthumb-interwork,\
+			$(call cc-option,-marm,)\
+			$(call cc-option,-mno-thumb-interwork,)\
+		)
+else
 PF_CPPFLAGS_ARM := $(call cc-option,-marm,)
+PF_CPPFLAGS_ARM += $(call cc-option,-mno-thumb-interwork,)
+endif
 
 # Try if EABI is supported, else fall back to old API,
 # i. e. for example:
 # - with ELDK 4.2 (EABI supported), use:
-#	-mabi=aapcs-linux -mno-thumb-interwork
+#	-mabi=aapcs-linux
 # - with ELDK 4.1 (gcc 4.x, no EABI), use:
-#	-mabi=apcs-gnu -mno-thumb-interwork
+#	-mabi=apcs-gnu
 # - with ELDK 3.1 (gcc 3.x), use:
-#	-mapcs-32 -mno-thumb-interwork
+#	-mapcs-32
 PF_CPPFLAGS_ABI := $(call cc-option,\
-			-mabi=aapcs-linux -mno-thumb-interwork,\
+			-mabi=aapcs-linux,\
 			$(call cc-option,\
 				-mapcs-32,\
 				$(call cc-option,\
 					-mabi=apcs-gnu,\
 				)\
-			) $(call cc-option,-mno-thumb-interwork,)\
+			)\
 		)
 PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_ARM) $(PF_CPPFLAGS_ABI)