| Message ID | 1300109258-12496-1-git-send-email-aneesh@ti.com |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
On Mon, Mar 14, 2011, Aneesh V wrote: > +- ARM Options: > + 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 > The above README changes suggest that "Thumb-1" would also be possible but you patch an armv7 file: > --- a/arch/arm/cpu/armv7/config.mk > +++ b/arch/arm/cpu/armv7/config.mk so I suspect only armv7 CPUs would be affected? Consider patching arch/arm/config.mk instead which is where -marm is currently set: # Explicitly specifiy 32-bit ARM ISA since toolchain default can be -mthumb: PLATFORM_CPPFLAGS += $(call cc-option,-marm,) In fact, I would find it less confusing if the flags were -mthumb or -marm and never -marm -mthumb, so you probably want to patch the code setting -marm above to be in your "else" clause. > @@ -31,3 +31,8 @@ PLATFORM_CPPFLAGS += -march=armv5 This suggests that u-boot is actually built in ARMv5 mode, which means we're missing out actual Thumb-2 instructions (v6+). Perhaps arch/arm/cpu/armv7/config.mk should also $(call cc-option,-march=armv7). (I've never used -march=armv7 but it seems to exist and is probably more correct than -march=armv7-a). > # ========================================================================= > PLATFORM_RELFLAGS +=$(call cc-option,-mshort-load-bytes,\ > $(call cc-option,-malignment-traps,)) > + > +ifeq ($(CONFIG_SYS_THUMB_BUILD),y) > +# Enable Thumb mode build if compiler supports it > +PLATFORM_CPPFLAGS += $(call cc-option,-mthumb -mthumb-interwork) > +endif > -- > 1.7.0.4 >
Loic, On Monday 14 March 2011 09:41 PM, Loïc Minier wrote: > On Mon, Mar 14, 2011, Aneesh V wrote: >> +- ARM Options: >> + 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 >> > > The above README changes suggest that "Thumb-1" would also be possible > but you patch an armv7 file: > >> --- a/arch/arm/cpu/armv7/config.mk >> +++ b/arch/arm/cpu/armv7/config.mk > > so I suspect only armv7 CPUs would be affected? Consider patching > arch/arm/config.mk instead which is where -marm is currently set: > # Explicitly specifiy 32-bit ARM ISA since toolchain default can be -mthumb: > PLATFORM_CPPFLAGS += $(call cc-option,-marm,) > In fact, I would find it less confusing if the flags were -mthumb or > -marm and never -marm -mthumb, so you probably want to patch the code > setting -marm above to be in your "else" clause. I will do that. Thank you. > >> @@ -31,3 +31,8 @@ PLATFORM_CPPFLAGS += -march=armv5 > > This suggests that u-boot is actually built in ARMv5 mode, which means > we're missing out actual Thumb-2 instructions (v6+). > > Perhaps arch/arm/cpu/armv7/config.mk should also $(call > cc-option,-march=armv7). (I've never used -march=armv7 but it seems to > exist and is probably more correct than -march=armv7-a). > Please note that I am enabling armv7-a in the second patch in omap4 config.mk file. The reason I didn't do this here was some ARMv7 SoCs do not want to use -march=armv7-a even if the compiler supports it. Tegra2 is an example. Please see the below from Tegra2 config.mk: # Use ARMv4 for Tegra2 - initial code runs on the AVP, which is an ARM7TDI. PLATFORM_CPPFLAGS += -march=armv4 This being the case I would have had to define another CONFIG flag if I had to add -march=armv7-a in arch/arm/cpu/armv7/config.mk. I thought it un-necessary and instead put it in the SoC specific file. So, Tegra2 can continue to use -march=armv4 and will get Thumb-1 if they enable CONFIG_SYS_THUMB_BUILD. Or do you think we should define something like CONFIG_SYS_MARCH_ARMV7 I will try -march=armv7, but I vaguely remember it had some issues. >> # ========================================================================= >> PLATFORM_RELFLAGS +=$(call cc-option,-mshort-load-bytes,\ >> $(call cc-option,-malignment-traps,)) >> + >> +ifeq ($(CONFIG_SYS_THUMB_BUILD),y) >> +# Enable Thumb mode build if compiler supports it >> +PLATFORM_CPPFLAGS += $(call cc-option,-mthumb -mthumb-interwork) >> +endif >> -- >> 1.7.0.4 >> >
On Tue, Mar 15, 2011, Aneesh V wrote: > Please note that I am enabling armv7-a in the second patch in omap4 > config.mk file. The reason I didn't do this here was some ARMv7 SoCs do > not want to use -march=armv7-a even if the compiler supports it. Tegra2 > is an example. Please see the below from Tegra2 config.mk: > > # Use ARMv4 for Tegra2 - initial code runs on the AVP, which is an ARM7TDI. > PLATFORM_CPPFLAGS += -march=armv4 Good point, I wonder whether it would make sense to have arch/arm/cpu/armv7/config.mk default to -march=armv7 and Tegra2 to override this with -march=armv4. Maybe this code doesn't belong under armv7 though; or perhaps -march=armv4 should only be set when building a subset of the files rather than by default. > This being the case I would have had to define another CONFIG flag if I > had to add -march=armv7-a in arch/arm/cpu/armv7/config.mk. I thought it > un-necessary and instead put it in the SoC specific file. So, Tegra2 > can continue to use -march=armv4 and will get Thumb-1 if they enable > CONFIG_SYS_THUMB_BUILD. Or do you think we should define something like > CONFIG_SYS_MARCH_ARMV7 Up to you, but I would expect that code udner arch/arm/cpu/armv7/ would build with -march=armv7 (maybe not -a though), with specific overrides where that's not the case; it would feel a bit odd to me to have this as a "config" option.
On Tuesday 15 March 2011 05:24 PM, Loïc Minier wrote: > On Tue, Mar 15, 2011, Aneesh V wrote: >> Please note that I am enabling armv7-a in the second patch in omap4 >> config.mk file. The reason I didn't do this here was some ARMv7 SoCs do >> not want to use -march=armv7-a even if the compiler supports it. Tegra2 >> is an example. Please see the below from Tegra2 config.mk: >> >> # Use ARMv4 for Tegra2 - initial code runs on the AVP, which is an ARM7TDI. >> PLATFORM_CPPFLAGS += -march=armv4 > > Good point, I wonder whether it would make sense to have > arch/arm/cpu/armv7/config.mk default to -march=armv7 and Tegra2 to Sounds reasonable. I will do that. > override this with -march=armv4. Maybe this code doesn't belong under > armv7 though; or perhaps -march=armv4 should only be set when building > a subset of the files rather than by default. I don't understand it either. Maybe, the early boot code runs on one processor and the rest run on another processor all in the same SoC. >> This being the case I would have had to define another CONFIG flag if I >> had to add -march=armv7-a in arch/arm/cpu/armv7/config.mk. I thought it >> un-necessary and instead put it in the SoC specific file. So, Tegra2 >> can continue to use -march=armv4 and will get Thumb-1 if they enable >> CONFIG_SYS_THUMB_BUILD. Or do you think we should define something like >> CONFIG_SYS_MARCH_ARMV7 > > Up to you, but I would expect that code udner arch/arm/cpu/armv7/ would > build with -march=armv7 (maybe not -a though), with specific overrides I tried -march=armv7 yesterday. I am getting several errors from the assembly files. Particularly, it doesn't support high registers, spsr etc. Looks like it defaults to the armv7-m variant(just my guess) > where that's not the case; it would feel a bit odd to me to have this > as a "config" option. Yes, me too didn't like having a CONFIG option for this. But that doesn't seem to be needed either as it can be over-ridden in the SoC directory. Albert, Your thoughts on this? best regards, Aneesh
Le 16/03/2011 09:39, Aneesh V a écrit : > On Tuesday 15 March 2011 05:24 PM, Loïc Minier wrote: >> On Tue, Mar 15, 2011, Aneesh V wrote: >>> Please note that I am enabling armv7-a in the second patch in omap4 >>> config.mk file. The reason I didn't do this here was some ARMv7 SoCs do >>> not want to use -march=armv7-a even if the compiler supports it. Tegra2 >>> is an example. Please see the below from Tegra2 config.mk: >>> >>> # Use ARMv4 for Tegra2 - initial code runs on the AVP, which is an >>> ARM7TDI. >>> PLATFORM_CPPFLAGS += -march=armv4 >> >> Good point, I wonder whether it would make sense to have >> arch/arm/cpu/armv7/config.mk default to -march=armv7 and Tegra2 to > > Sounds reasonable. I will do that. > >> override this with -march=armv4. Maybe this code doesn't belong under >> armv7 though; or perhaps -march=armv4 should only be set when building >> a subset of the files rather than by default. > > I don't understand it either. Maybe, the early boot code runs on one > processor and the rest run on another processor all in the same SoC. > >>> This being the case I would have had to define another CONFIG flag if I >>> had to add -march=armv7-a in arch/arm/cpu/armv7/config.mk. I thought it >>> un-necessary and instead put it in the SoC specific file. So, Tegra2 >>> can continue to use -march=armv4 and will get Thumb-1 if they enable >>> CONFIG_SYS_THUMB_BUILD. Or do you think we should define something like >>> CONFIG_SYS_MARCH_ARMV7 >> >> Up to you, but I would expect that code udner arch/arm/cpu/armv7/ would >> build with -march=armv7 (maybe not -a though), with specific overrides > > I tried -march=armv7 yesterday. I am getting several errors from the > assembly files. Particularly, it doesn't support high registers, spsr > etc. Looks like it defaults to the armv7-m variant(just my guess) > >> where that's not the case; it would feel a bit odd to me to have this >> as a "config" option. > > Yes, me too didn't like having a CONFIG option for this. But that > doesn't seem to be needed either as it can be over-ridden in the SoC > directory. > > Albert, > Your thoughts on this? Some toolchains in current use (notably the one provided with ELDK 4.2, and possibly others) do not support -march=armv7[-a]. Do we really need armv7 instructions? > best regards, > Aneesh Amicalement,
Albert, On Wednesday 16 March 2011 10:55 PM, Albert ARIBAUD wrote: > Le 16/03/2011 09:39, Aneesh V a écrit : [snip ..] > > Some toolchains in current use (notably the one provided with ELDK 4.2, > and possibly others) do not support -march=armv7[-a]. Do we really need > armv7 instructions? 'cc-option' will make sure that build won't break for old compilers. We can do something like this: PLATFORM_CPPFLAGS += $(call cc-option,-march=armv7-a, -march=armv5) We are not planning to use armv7-a instructions in the code. In fact, recently I removed even an armv5 instruction from armv7 generic code in the interest of not breaking Tegra2. However, keeping -march=armv7-a (armv6 and above) helps in having Thumb2, which I believe is a good compromise between code density and performance. So, OMAP4 U-Boot when built with ELDK will give you Thumb1 code where as when it is built with newer compilers it will give you Thumb2 code. As long we don't use non-compliant assembly instructions, both should build and work fine. br, Aneesh
Le 17/03/2011 08:30, Aneesh V a écrit : > Albert, > > On Wednesday 16 March 2011 10:55 PM, Albert ARIBAUD wrote: >> Le 16/03/2011 09:39, Aneesh V a écrit : > [snip ..] >> >> Some toolchains in current use (notably the one provided with ELDK 4.2, >> and possibly others) do not support -march=armv7[-a]. Do we really need >> armv7 instructions? > > 'cc-option' will make sure that build won't break for old compilers. We > can do something like this: > > PLATFORM_CPPFLAGS += $(call cc-option,-march=armv7-a, -march=armv5) > > We are not planning to use armv7-a instructions in the code. In fact, > recently I removed even an armv5 instruction from armv7 generic code in > the interest of not breaking Tegra2. > > However, keeping -march=armv7-a (armv6 and above) helps in having > Thumb2, which I believe is a good compromise between code density and > performance. > > So, OMAP4 U-Boot when built with ELDK will give you Thumb1 code where > as when it is built with newer compilers it will give you Thumb2 code. > As long we don't use non-compliant assembly instructions, both should > build and work fine. > > br, > Aneesh Understood. As for the question about config options (assuming I got it right) I'd rather have the SoC define which -march it wants, and only have a CONFIG for saying if we want thumb or not. Amicalement,
diff --git a/README b/README index f1547a4..ff3a345 100644 --- a/README +++ b/README @@ -370,6 +370,15 @@ The following options need to be configured: 2. The core frequency as calculated above is multiplied by this value. +- ARM Options: + 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/cpu/armv7/config.mk b/arch/arm/cpu/armv7/config.mk index 49ac9c7..b7bebad 100644 --- a/arch/arm/cpu/armv7/config.mk +++ b/arch/arm/cpu/armv7/config.mk @@ -31,3 +31,8 @@ PLATFORM_CPPFLAGS += -march=armv5 # ========================================================================= PLATFORM_RELFLAGS +=$(call cc-option,-mshort-load-bytes,\ $(call cc-option,-malignment-traps,)) + +ifeq ($(CONFIG_SYS_THUMB_BUILD),y) +# Enable Thumb mode build if compiler supports it +PLATFORM_CPPFLAGS += $(call cc-option,-mthumb -mthumb-interwork) +endif
Signed-off-by: Aneesh V <aneesh@ti.com> --- README | 9 +++++++++ arch/arm/cpu/armv7/config.mk | 5 +++++ 2 files changed, 14 insertions(+), 0 deletions(-)