fix build failure on arm because of missing clz instruction

Submitted by Szabolcs Nagy on Aug. 24, 2018, 7:30 p.m.

Details

Message ID 20180824193052.GD4418@port70.net
State New
Series "fix build failure on arm because of missing clz instruction"
Headers show

Commit Message

Szabolcs Nagy Aug. 24, 2018, 7:30 p.m.
another arm patch, clz usage (in fma) was broken with -mthumb -march=armv5t.

Patch hide | download patch | download mbox

From b6036dd2256edca0181aa25de2259bcd03b21c2e Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <nsz@port70.net>
Date: Fri, 24 Aug 2018 15:59:17 +0000
Subject: [PATCH] fix build failure on arm because of missing clz instruction

In thumb mode clz is only available since armv6t2, in arm mode it is
available since armv5.

The preprocessor conditionals are changed accordingly.
---
 arch/arm/atomic_arch.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/atomic_arch.h b/arch/arm/atomic_arch.h
index 62458b45..868e5758 100644
--- a/arch/arm/atomic_arch.h
+++ b/arch/arm/atomic_arch.h
@@ -82,7 +82,8 @@  static inline void a_crash()
 		: : : "memory");
 }
 
-#if __ARM_ARCH >= 5
+#if __ARM_ARCH_6T2__ || __ARM_ARCH_7A__ || __ARM_ARCH_7R__ || __ARM_ARCH >= 7 \
+ || (__ARM_ARCH >= 5 && !__thumb__)
 
 #define a_clz_32 a_clz_32
 static inline int a_clz_32(uint32_t x)
-- 
2.18.0


Comments

Andre McCurdy Aug. 24, 2018, 9:58 p.m.
On Fri, Aug 24, 2018 at 12:30 PM, Szabolcs Nagy <nsz@port70.net> wrote:
> another arm patch, clz usage (in fma) was broken with -mthumb -march=armv5t.

That conditional was originally written under the assumption that musl
doesn't support thumb1 (so -mthumb -march=armv5t is not a supported
configuration).

Was that assumption wrong?
Andre McCurdy Aug. 24, 2018, 10:04 p.m.
On Fri, Aug 24, 2018 at 2:58 PM, Andre McCurdy <armccurdy@gmail.com> wrote:
> On Fri, Aug 24, 2018 at 12:30 PM, Szabolcs Nagy <nsz@port70.net> wrote:
>> another arm patch, clz usage (in fma) was broken with -mthumb -march=armv5t.
>
> That conditional was originally written under the assumption that musl
> doesn't support thumb1 (so -mthumb -march=armv5t is not a supported
> configuration).
>
> Was that assumption wrong?

Digging back through the mailing list where this was discussed before:

  http://www.openwall.com/lists/musl/2017/10/20/10
Szabolcs Nagy Aug. 24, 2018, 10:05 p.m.
* Andre McCurdy <armccurdy@gmail.com> [2018-08-24 14:58:04 -0700]:
> On Fri, Aug 24, 2018 at 12:30 PM, Szabolcs Nagy <nsz@port70.net> wrote:
> > another arm patch, clz usage (in fma) was broken with -mthumb -march=armv5t.
> 
> That conditional was originally written under the assumption that musl
> doesn't support thumb1 (so -mthumb -march=armv5t is not a supported
> configuration).
> 
> Was that assumption wrong?

the asm code in musl does not support thumb1,
but you can compile everything else with thumb1
(the compiler does not pass -mthumb to the assembler
by default so all asm code will be in arm mode)

so with the changed condition the build succeeds
with -mthumb -march=armv5t, but the resulting libc
will not be competely thumb code, i think that's
an improvement.
Andre McCurdy Aug. 24, 2018, 10:50 p.m.
On Fri, Aug 24, 2018 at 3:05 PM, Szabolcs Nagy <nsz@port70.net> wrote:
> * Andre McCurdy <armccurdy@gmail.com> [2018-08-24 14:58:04 -0700]:
>> On Fri, Aug 24, 2018 at 12:30 PM, Szabolcs Nagy <nsz@port70.net> wrote:
>> > another arm patch, clz usage (in fma) was broken with -mthumb -march=armv5t.
>>
>> That conditional was originally written under the assumption that musl
>> doesn't support thumb1 (so -mthumb -march=armv5t is not a supported
>> configuration).
>>
>> Was that assumption wrong?
>
> the asm code in musl does not support thumb1,
> but you can compile everything else with thumb1
> (the compiler does not pass -mthumb to the assembler
> by default so all asm code will be in arm mode)
>
> so with the changed condition the build succeeds
> with -mthumb -march=armv5t, but the resulting libc
> will not be competely thumb code, i think that's
> an improvement.

Yes, I agree it's an improvement. Mostly thumb1 with a little ARM is
still useful for a lot of use cases.

As an aside, it seems that glibc unconditionally includes some thumb2
when targeting ARMv7, so there's precedent for mixing ARM and
thumb[12] in libc.

  https://sourceware.org/bugzilla/show_bug.cgi?id=23031#c15
Rich Felker Aug. 24, 2018, 11:20 p.m.
On Fri, Aug 24, 2018 at 02:58:04PM -0700, Andre McCurdy wrote:
> On Fri, Aug 24, 2018 at 12:30 PM, Szabolcs Nagy <nsz@port70.net> wrote:
> > another arm patch, clz usage (in fma) was broken with -mthumb -march=armv5t.
> 
> That conditional was originally written under the assumption that musl
> doesn't support thumb1 (so -mthumb -march=armv5t is not a supported
> configuration).
> 
> Was that assumption wrong?

musl does not support being pure-thumb1 code, because some of the asm
source files are not thumb-compatible, but I think the C code can be
compiled as thumb1. -mthumb is only passed to the assembler for asm
source files if __thumb2__ is defined.

Rich