| Message ID | 1441470824-23456-1-git-send-email-public.douglas.raillard@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
Dear Douglas RAILLARD, On Sat, 5 Sep 2015 18:33:44 +0200, Douglas RAILLARD wrote: > +# UCLIBC_HAS_CONTEXT_FUNCS is broken with Thumb and Thumb2 > +ifeq (y,$(filter y,$(BR2_ARM_INSTRUCTIONS_THUMB) $(BR2_ARM_INSTRUCTIONS_THUMB2))) Why such a complicated condition? BR2_ARM_INSTRUCTIONS_THUMB and BR2_ARM_INSTRUCTIONS_THUMB2 are in a "choice" ... "endchoice" block, so only one of the two can be set to 'y' at any given time. So you can do: ifeq ($(BR2_ARM_INSTRUCTIONS_THUMB)$(BR2_ARM_INSTRUCTIONS_THUMB2),y) instead. > +define UCLIBC_ARM_THUMB_CONFIG > + $(call KCONFIG_ENABLE_OPT,COMPILE_IN_THUMB_MODE,$(@D)/.config) > + $(call KCONFIG_DISABLE_OPT,UCLIBC_HAS_CONTEXT_FUNCS,$(@D)/.config) > +endef > +endif Other than that, it looks good to me. Thanks, Thomas
On 06/09/2015 15:25, Thomas Petazzoni wrote: > Dear Douglas RAILLARD, > > On Sat, 5 Sep 2015 18:33:44 +0200, Douglas RAILLARD wrote: > >> +# UCLIBC_HAS_CONTEXT_FUNCS is broken with Thumb and Thumb2 >> +ifeq (y,$(filter y,$(BR2_ARM_INSTRUCTIONS_THUMB) $(BR2_ARM_INSTRUCTIONS_THUMB2))) > > Why such a complicated condition? BR2_ARM_INSTRUCTIONS_THUMB and > BR2_ARM_INSTRUCTIONS_THUMB2 are in a "choice" ... "endchoice" block, so > only one of the two can be set to 'y' at any given time. So you can do: > > ifeq ($(BR2_ARM_INSTRUCTIONS_THUMB)$(BR2_ARM_INSTRUCTIONS_THUMB2),y) > > instead. Right, a general OR is not needed here >> +define UCLIBC_ARM_THUMB_CONFIG >> + $(call KCONFIG_ENABLE_OPT,COMPILE_IN_THUMB_MODE,$(@D)/.config) >> + $(call KCONFIG_DISABLE_OPT,UCLIBC_HAS_CONTEXT_FUNCS,$(@D)/.config) >> +endef >> +endif > > Other than that, it looks good to me. One thing that this patch does not reflect is that the only threading backend which works (and compiles) on ARM Thumb is BR2_PTHREADS_OLD. Should we warn the user or is its responsibility to choose what is appropriate for this target ? Thanks for the comments ! Douglas
Douglas, On Sun, 6 Sep 2015 17:35:05 +0200, Douglas RAILLARD wrote: > >> +define UCLIBC_ARM_THUMB_CONFIG > >> + $(call KCONFIG_ENABLE_OPT,COMPILE_IN_THUMB_MODE,$(@D)/.config) > >> + $(call KCONFIG_DISABLE_OPT,UCLIBC_HAS_CONTEXT_FUNCS,$(@D)/.config) > >> +endef > >> +endif > > > > Other than that, it looks good to me. > > One thing that this patch does not reflect is that the only threading > backend which works (and compiles) on ARM Thumb is BR2_PTHREADS_OLD. > Should we warn the user or is its responsibility to choose what is > appropriate for this target ? No, we have to make sure that the user cannot do a selection that is known to not work. This problem is already partially handled by the following piece of code in uclibc.mk: # Thumb build is broken with threads, build in ARM mode ifeq ($(BR2_ARM_INSTRUCTIONS_THUMB)$(BR2_TOOLCHAIN_HAS_THREADS),yy) UCLIBC_EXTRA_CFLAGS += -marm endif But this only handles the case of ARM platforms that support the original Thumb instruction set, and which also support the regular ARM instruction set. This is for example the case on ARMv4 or ARMv5 (possibly ARMv6, but I'm not sure). The Cortex-M3 is defined in Buildroot as: config BR2_cortex_m3 bool "cortex-M3" select BR2_ARM_CPU_HAS_THUMB select BR2_ARM_CPU_HAS_THUMB2 However, I am not sure this makes sense. Gcc only understands a -mthumb option, which means Thumb on ARMv4/v5, and Thumb-2 on ARMv7 (at least for ARMv7-A). So I believe we should remove the "select BR2_ARM_CPU_HAS_THUMB" from this BR2_cortex_m3 option. Then, we should test if building uclibc in Thumb 2 mode on ARMv7-A with NPTL enabled works. Depending on whether it works or not, the choice will be different. If it doesn't work, and indeed building the NPTL code in Thumb-2 is broken, then we will have to: 1/ Enforce building with the ARM instruction set on ARMv7-A 2/ Prevent from selecting NPTL as the thread implementation on ARMv7-M Does that make sense? Thanks! Thomas
Thomas On 06/09/2015 18:01, Thomas Petazzoni wrote: > However, I am not sure this makes sense. Gcc only understands a -mthumb > option, which means Thumb on ARMv4/v5, and Thumb-2 on ARMv7 (at least > for ARMv7-A). So I believe we should remove the "select > BR2_ARM_CPU_HAS_THUMB" from this BR2_cortex_m3 option. There does not seem to be a way to force gcc to generate Thumb instead of Thumb2 on ARMv7-M, so in reality, Cortex-M3 only supports Thumb2 when using gcc toolchain. However, BR2_PTHREADS_OLD does compile with Thumb2. If it compiles for Thumb too, the test on BR2_TOOLCHAIN_HAS_THREADS is maybe too generic, it should be on BR2_PTHREADS_NATIVE instead. We should then test BR2_PTHREADS_OLD with a true Thumb target (i.e. where -mthumb generates Thumb and not Thumb2). If that works, we should probably replace: ifeq ($(BR2_ARM_INSTRUCTIONS_THUMB)$(BR2_TOOLCHAIN_HAS_THREADS),yy) with ifeq ($(BR2_ARM_INSTRUCTIONS_THUMB)$(BR2_PTHREADS_NATIVE),yy) or something like that. > Then, we should test if building uclibc in Thumb 2 mode on ARMv7-A with > NPTL enabled works. Depending on whether it works or not, the choice > will be different. If it doesn't work, and indeed building the NPTL > code in Thumb-2 is broken, then we will have to: > > 1/ Enforce building with the ARM instruction set on ARMv7-A Building for cortex-A8 with BR2_ARM_INSTRUCTIONS_THUMB2 selected and BR2_PTHREADS_NATIVE works. I checked that the uClibc configuration was set for Thumb and NPTL accordingly. This is strange because Waldemar said that NPTL is not Thumb ready yet, and it fails when building for Cortex-M3. My guess is that inline ARM assembly used in uClibc is given to gas as is, so as long as the target can handle ARM, gas happily output ARM instructions, even if gcc has generated Thumb2 assembly for the C code, but it is only speculations. > 2/ Prevent from selecting NPTL as the thread implementation on ARMv7-M Is there an already implemented way of telling if ARMv7-M target is selected ? Testing BR2_ARM_INSTRUCTIONS_THUMB2 is not enough because NPTL builds on Cortex-A8 with BR2_ARM_INSTRUCTIONS_THUMB2. Best, Douglas
diff --git a/package/uclibc/uclibc.mk b/package/uclibc/uclibc.mk index a2ba230..116efb1 100644 --- a/package/uclibc/uclibc.mk +++ b/package/uclibc/uclibc.mk @@ -94,6 +94,14 @@ ifeq ($(BR2_ARM_INSTRUCTIONS_THUMB)$(BR2_TOOLCHAIN_HAS_THREADS),yy) UCLIBC_EXTRA_CFLAGS += -marm endif +# UCLIBC_HAS_CONTEXT_FUNCS is broken with Thumb and Thumb2 +ifeq (y,$(filter y,$(BR2_ARM_INSTRUCTIONS_THUMB) $(BR2_ARM_INSTRUCTIONS_THUMB2))) +define UCLIBC_ARM_THUMB_CONFIG + $(call KCONFIG_ENABLE_OPT,COMPILE_IN_THUMB_MODE,$(@D)/.config) + $(call KCONFIG_DISABLE_OPT,UCLIBC_HAS_CONTEXT_FUNCS,$(@D)/.config) +endef +endif + ifeq ($(BR2_UCLIBC_ARM_BX),y) define UCLIBC_ARM_BX_CONFIG $(call KCONFIG_ENABLE_OPT,USE_BX,$(@D)/.config) @@ -376,6 +384,7 @@ define UCLIBC_KCONFIG_FIXUP_CMDS $(UCLIBC_ARC_TYPE_CONFIG) $(UCLIBC_ARC_PAGE_SIZE_CONFIG) $(UCLIBC_ARM_ABI_CONFIG) + $(UCLIBC_ARM_THUMB_CONFIG) $(UCLIBC_ARM_BX_CONFIG) $(UCLIBC_MIPS_ABI_CONFIG) $(UCLIBC_MIPS_ISA_CONFIG)
* Use Thumb in uClibc when targeting Thumb(2) in Buildroot * Disable context functions that are not Thumb ready Signed-off-by: Douglas RAILLARD <public.douglas.raillard@gmail.com> --- package/uclibc/uclibc.mk | 9 +++++++++ 1 file changed, 9 insertions(+)