diff mbox

[v2,08/17] uclibc: context functions are not Thumb-compatible

Message ID 1458335299-27409-9-git-send-email-thomas.petazzoni@free-electrons.com
State Superseded
Headers show

Commit Message

Thomas Petazzoni March 18, 2016, 9:08 p.m. UTC
On Thumb1 platforms and Thumb2 platforms that support the ARM
instructions set, there is no problem, as the context functions are
built unconditionally in ARM mode.

However, on Thumb2 platforms that only support the Thumb2 instruction
set, the context functions cannot build as the assembler code is not
Thumb-ready. Therefore, those functions must be disabled.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/uclibc/uclibc.mk | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Arnout Vandecappelle March 19, 2016, 4:20 p.m. UTC | #1
On 03/18/16 22:08, Thomas Petazzoni wrote:
> On Thumb1 platforms and Thumb2 platforms that support the ARM

  This sentence is a bit weird, unless you realise that we don't have any thumb1 
platforms that don't have ARM instructions.

> instructions set, there is no problem, as the context functions are
> built unconditionally in ARM mode.
>
> However, on Thumb2 platforms that only support the Thumb2 instruction
> set, the context functions cannot build as the assembler code is not
> Thumb-ready. Therefore, those functions must be disabled.

  It may be useful to note that this will break packages that rely on the 
context functions, so that these packages must be disabled for that architecture 
combination.

  BTW, for that reason, it may be better to create a Kconfig blind option to 
encode this situation.

  In summary, a better commit message could be:

uclibc: context functions are not Thumb-compatible

On platforms that do support the ARM instruction set, there is no problem, as 
the context functions are built unconditionally in ARM mode.

However, on platforms that only support the Thumb instruction set, the context 
functions cannot be built since the assembler code is not Thumb-ready. 
Therefore, these functions must be disabled. All Thumb1 platforms support ARM 
instructions, so this is only relevant for Thumb2 platforms (i.e., Cortex-M).

Note that some packages require the context functions, so these will fail to 
build on these platforms. Those will be handled in later patches.


  Regards,
  Arnout

>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>   package/uclibc/uclibc.mk | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/package/uclibc/uclibc.mk b/package/uclibc/uclibc.mk
> index e3e45f7..4698132 100644
> --- a/package/uclibc/uclibc.mk
> +++ b/package/uclibc/uclibc.mk
> @@ -88,6 +88,12 @@ define UCLIBC_ARM_BINFMT_FLAT
>   endef
>   endif
>
> +ifeq ($(BR2_ARM_INSTRUCTIONS_THUMB2):$(BR2_ARM_CPU_HAS_ARM),y:)
> +define UCLIBC_ARM_NO_CONTEXT_FUNCS
> +	$(call KCONFIG_DISABLE_OPT,UCLIBC_HAS_CONTEXT_FUNCS,$(@D)/.config)
> +endef
> +endif
> +
>   endif # arm
>
>   #
> @@ -361,6 +367,7 @@ define UCLIBC_KCONFIG_FIXUP_CMDS
>   	$(UCLIBC_ARC_PAGE_SIZE_CONFIG)
>   	$(UCLIBC_ARM_ABI_CONFIG)
>   	$(UCLIBC_ARM_BINFMT_FLAT)
> +	$(UCLIBC_ARM_NO_CONTEXT_FUNCS)
>   	$(UCLIBC_MIPS_ABI_CONFIG)
>   	$(UCLIBC_MIPS_ISA_CONFIG)
>   	$(UCLIBC_SH_TYPE_CONFIG)
>
Thomas Petazzoni March 19, 2016, 6:07 p.m. UTC | #2
Hello,

On Sat, 19 Mar 2016 17:20:09 +0100, Arnout Vandecappelle wrote:
> On 03/18/16 22:08, Thomas Petazzoni wrote:
> > On Thumb1 platforms and Thumb2 platforms that support the ARM
> 
>   This sentence is a bit weird, unless you realise that we don't have any thumb1 
> platforms that don't have ARM instructions.

Right.

>   It may be useful to note that this will break packages that rely on the 
> context functions, so that these packages must be disabled for that architecture 
> combination.

Correct, but we already have such a case with musl, which doesn't
implement the context functions.

>   BTW, for that reason, it may be better to create a Kconfig blind option to 
> encode this situation.

I think we need to see how many packages use such functions. Probably
not many.

> uclibc: context functions are not Thumb-compatible
> 
> On platforms that do support the ARM instruction set, there is no problem, as 
> the context functions are built unconditionally in ARM mode.
> 
> However, on platforms that only support the Thumb instruction set, the context 
> functions cannot be built since the assembler code is not Thumb-ready. 
> Therefore, these functions must be disabled. All Thumb1 platforms support ARM 
> instructions, so this is only relevant for Thumb2 platforms (i.e., Cortex-M).

I would do: "only relevant for Thumb2-only platforms". Cortex-A is a
Thumb2 platform, but does not have any problem as it also supports the
ARM instruction set.

Thanks!

Thomas
Arnout Vandecappelle March 20, 2016, 7:39 p.m. UTC | #3
On 03/19/16 19:07, Thomas Petazzoni wrote:
> Hello,
>
> On Sat, 19 Mar 2016 17:20:09 +0100, Arnout Vandecappelle wrote:
[snip]
>> uclibc: context functions are not Thumb-compatible
>>
>> On platforms that do support the ARM instruction set, there is no problem, as
>> the context functions are built unconditionally in ARM mode.
>>
>> However, on platforms that only support the Thumb instruction set, the context
>> functions cannot be built since the assembler code is not Thumb-ready.
>> Therefore, these functions must be disabled. All Thumb1 platforms support ARM
>> instructions, so this is only relevant for Thumb2 platforms (i.e., Cortex-M).
>
> I would do: "only relevant for Thumb2-only platforms". Cortex-A is a
> Thumb2 platform, but does not have any problem as it also supports the
> ARM instruction set.

  Right!

  Regards,
  Arnout
diff mbox

Patch

diff --git a/package/uclibc/uclibc.mk b/package/uclibc/uclibc.mk
index e3e45f7..4698132 100644
--- a/package/uclibc/uclibc.mk
+++ b/package/uclibc/uclibc.mk
@@ -88,6 +88,12 @@  define UCLIBC_ARM_BINFMT_FLAT
 endef
 endif
 
+ifeq ($(BR2_ARM_INSTRUCTIONS_THUMB2):$(BR2_ARM_CPU_HAS_ARM),y:)
+define UCLIBC_ARM_NO_CONTEXT_FUNCS
+	$(call KCONFIG_DISABLE_OPT,UCLIBC_HAS_CONTEXT_FUNCS,$(@D)/.config)
+endef
+endif
+
 endif # arm
 
 #
@@ -361,6 +367,7 @@  define UCLIBC_KCONFIG_FIXUP_CMDS
 	$(UCLIBC_ARC_PAGE_SIZE_CONFIG)
 	$(UCLIBC_ARM_ABI_CONFIG)
 	$(UCLIBC_ARM_BINFMT_FLAT)
+	$(UCLIBC_ARM_NO_CONTEXT_FUNCS)
 	$(UCLIBC_MIPS_ABI_CONFIG)
 	$(UCLIBC_MIPS_ISA_CONFIG)
 	$(UCLIBC_SH_TYPE_CONFIG)