[PATCH] fix build failure on arm because of missing clz instruction

[PATCH] fix build failure on arm because of missing clz instruction

[Szabolcs Nagy]

[-- Attachment #1: Type: text/plain, Size: 77 bytes --]

another arm patch, clz usage (in fma) was broken with -mthumb -march=armv5t.

[-- Attachment #2: 0001-fix-build-failure-on-arm-because-of-missing-clz-inst.patch --]
[-- Type: text/x-diff, Size: 900 bytes --]

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

Re: [PATCH] fix build failure on arm because of missing clz instruction

[Andre McCurdy]

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?

Re: [PATCH] fix build failure on arm because of missing clz instruction

[Andre McCurdy]

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

Re: [PATCH] fix build failure on arm because of missing clz instruction

[Szabolcs Nagy]

* 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.

Re: [PATCH] fix build failure on arm because of missing clz instruction

[Andre McCurdy]

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

Re: [PATCH] fix build failure on arm because of missing clz instruction

[Rich Felker]

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

Re: [PATCH] fix build failure on arm because of missing clz instruction

[Andre McCurdy]

On Fri, Aug 24, 2018 at 4:20 PM Rich Felker <dalias@libc.org> wrote:
> 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.

Sorry to resurrect such an old thread, but it seems this patch was
never applied?

Without it, -mthumb -march=armv5t still fails to build due to clz
getting into C code via inline assembler.

Re: [PATCH] fix build failure on arm because of missing clz instruction

[Rich Felker]

On Fri, Jun 28, 2019 at 03:55:56PM -0700, Andre McCurdy wrote:
> On Fri, Aug 24, 2018 at 4:20 PM Rich Felker <dalias@libc.org> wrote:
> > 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.
> 
> Sorry to resurrect such an old thread, but it seems this patch was
> never applied?
> 
> Without it, -mthumb -march=armv5t still fails to build due to clz
> getting into C code via inline assembler.

Thanks for reviving this thread. I'll commit it or something similar.
I wonder if _ARM_ARCH>=5 && __thumb__!=1 would be a better test.

Rich

Re: [PATCH] fix build failure on arm because of missing clz instruction

[Andre McCurdy]

On Fri, Jun 28, 2019 at 9:19 PM Rich Felker <dalias@libc.org> wrote:
> On Fri, Jun 28, 2019 at 03:55:56PM -0700, Andre McCurdy wrote:
> > On Fri, Aug 24, 2018 at 4:20 PM Rich Felker <dalias@libc.org> wrote:
> > > 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.
> >
> > Sorry to resurrect such an old thread, but it seems this patch was
> > never applied?
> >
> > Without it, -mthumb -march=armv5t still fails to build due to clz
> > getting into C code via inline assembler.
>
> Thanks for reviving this thread. I'll commit it or something similar.
> I wonder if _ARM_ARCH>=5 && __thumb__!=1 would be a better test.

__thumb__ is either 1 (for both Thumb1 and Thumb2) or undefined (for
ARM). The above might misleadingly suggest that it's sometimes defined
with a value other than 1?

Re: [PATCH] fix build failure on arm because of missing clz instruction

[Rich Felker]

On Mon, Jul 01, 2019 at 12:09:26PM -0700, Andre McCurdy wrote:
> On Fri, Jun 28, 2019 at 9:19 PM Rich Felker <dalias@libc.org> wrote:
> > On Fri, Jun 28, 2019 at 03:55:56PM -0700, Andre McCurdy wrote:
> > > On Fri, Aug 24, 2018 at 4:20 PM Rich Felker <dalias@libc.org> wrote:
> > > > 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.
> > >
> > > Sorry to resurrect such an old thread, but it seems this patch was
> > > never applied?
> > >
> > > Without it, -mthumb -march=armv5t still fails to build due to clz
> > > getting into C code via inline assembler.
> >
> > Thanks for reviving this thread. I'll commit it or something similar.
> > I wonder if _ARM_ARCH>=5 && __thumb__!=1 would be a better test.
> 
> __thumb__ is either 1 (for both Thumb1 and Thumb2) or undefined (for
> ARM). The above might misleadingly suggest that it's sometimes defined
> with a value other than 1?

Oh, sorry. I meant _ARM_ARCH>=5 && (!__thumb__ || __thumb2__). I was
wrongly thinking __thumb__ was defined as 1/2 rather than having a
separate __thumb2__ macro.

Rich

Re: [PATCH] fix build failure on arm because of missing clz instruction

[Andre McCurdy]

On Mon, Jul 1, 2019 at 1:10 PM Rich Felker <dalias@libc.org> wrote:
> On Mon, Jul 01, 2019 at 12:09:26PM -0700, Andre McCurdy wrote:
> > On Fri, Jun 28, 2019 at 9:19 PM Rich Felker <dalias@libc.org> wrote:
> > > On Fri, Jun 28, 2019 at 03:55:56PM -0700, Andre McCurdy wrote:
> > > > On Fri, Aug 24, 2018 at 4:20 PM Rich Felker <dalias@libc.org> wrote:
> > > > > 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.
> > > >
> > > > Sorry to resurrect such an old thread, but it seems this patch was
> > > > never applied?
> > > >
> > > > Without it, -mthumb -march=armv5t still fails to build due to clz
> > > > getting into C code via inline assembler.
> > >
> > > Thanks for reviving this thread. I'll commit it or something similar.
> > > I wonder if _ARM_ARCH>=5 && __thumb__!=1 would be a better test.
> >
> > __thumb__ is either 1 (for both Thumb1 and Thumb2) or undefined (for
> > ARM). The above might misleadingly suggest that it's sometimes defined
> > with a value other than 1?
>
> Oh, sorry. I meant _ARM_ARCH>=5 && (!__thumb__ || __thumb2__). I was
> wrongly thinking __thumb__ was defined as 1/2 rather than having a
> separate __thumb2__ macro.

I think that version looks OK. Note that __ARM_ARCH has two leading underscores.