[compiler-rt] r219040 - Fix the armv7 thumb builtins on darwin

Saleem Abdulrasool compnerd at compnerd.org
Mon Oct 6 18:29:21 PDT 2014


On Mon, Oct 6, 2014 at 6:24 PM, Steven Wu <stevenwu at apple.com> wrote:

> Agree. I made the patch that way because .thumb directive is separate from
> function declaration and it is hard to figure out whether some function is
> arm or thumb.
> You can wrap #ifdef __ARM_ARCH_ISA_THUMB == 2 around the function
> declaration to pick DEFINE_COMPILERRT_FUNCTION or
> DEFINE_COMPILER_THUMB_FUNCTION to make the code cleaner, but it will make
> no functional change.
> I am working on a patch to convert all arm builtins to Thumb if possible.
> After that, we should have only one function declaration macro in both
> cases.
>

Agreed that it is no functional change.  I like this suggestion, and think
I will go ahead and make that change.

I will add that I look forward to the conversion.  I will admit that I
didn't spend any time to check if anything could be better optimized with
the thumb instructions (as perhaps I should have).

Thanks for working on this!

> On Oct 6, 2014, at 6:17 PM, Saleem Abdulrasool <compnerd at compnerd.org>
> wrote:
>
>
>
> On Fri, Oct 3, 2014 at 5:18 PM, Steven Wu <stevenwu at apple.com> wrote:
>
>> Author: steven_wu
>> Date: Fri Oct  3 19:18:59 2014
>> New Revision: 219040
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=219040&view=rev
>> Log:
>> Fix the armv7 thumb builtins on darwin
>>
>> The arm builtins converted into thumb in r213481 are not working
>> on darwin. On apple platforms, .thumb_func directive is required
>> to generated correct symbols for thumb functions.
>>
>> <rdar://problem/18523605>
>>
>> Modified:
>>     compiler-rt/trunk/lib/builtins/arm/bswapdi2.S
>>     compiler-rt/trunk/lib/builtins/arm/bswapsi2.S
>>     compiler-rt/trunk/lib/builtins/arm/clzdi2.S
>>     compiler-rt/trunk/lib/builtins/arm/clzsi2.S
>>     compiler-rt/trunk/lib/builtins/arm/divmodsi4.S
>>     compiler-rt/trunk/lib/builtins/arm/divsi3.S
>>     compiler-rt/trunk/lib/builtins/arm/modsi3.S
>>     compiler-rt/trunk/lib/builtins/arm/sync-ops.h
>>     compiler-rt/trunk/lib/builtins/arm/udivmodsi4.S
>>     compiler-rt/trunk/lib/builtins/arm/udivsi3.S
>>     compiler-rt/trunk/lib/builtins/arm/umodsi3.S
>>     compiler-rt/trunk/lib/builtins/assembly.h
>>
>
> Sorry to resurrect this thread again, but, this makes the code harder to
> follow IMO.  You are now using DEFINE_COMPILERRT_THUMB_FUNCTION to declare
> a function that may actually be an ARM function by sinking a similar check
> to the .thumb into the header.
>
> What happens if someone uses a different check for the .thumb and then
> uses the THUMB_FUNCTION macro?  Or doesn't use it?  I think that it might
> be better to hoist the .thumb_func into the implementation so that it is
> more immediately obvious.
>
>
>> Modified: compiler-rt/trunk/lib/builtins/arm/bswapdi2.S
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/arm/bswapdi2.S?rev=219040&r1=219039&r2=219040&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/builtins/arm/bswapdi2.S (original)
>> +++ compiler-rt/trunk/lib/builtins/arm/bswapdi2.S Fri Oct  3 19:18:59 2014
>> @@ -21,7 +21,7 @@
>>  // Reverse all the bytes in a 64-bit integer.
>>  //
>>         .p2align 2
>> -DEFINE_COMPILERRT_FUNCTION(__bswapdi2)
>> +DEFINE_COMPILERRT_THUMB_FUNCTION(__bswapdi2)
>>  #if __ARM_ARCH < 6
>>      // before armv6 does not have "rev" instruction
>>      // r2 = rev(r0)
>>
>> Modified: compiler-rt/trunk/lib/builtins/arm/bswapsi2.S
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/arm/bswapsi2.S?rev=219040&r1=219039&r2=219040&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/builtins/arm/bswapsi2.S (original)
>> +++ compiler-rt/trunk/lib/builtins/arm/bswapsi2.S Fri Oct  3 19:18:59 2014
>> @@ -21,7 +21,7 @@
>>  // Reverse all the bytes in a 32-bit integer.
>>  //
>>         .p2align 2
>> -DEFINE_COMPILERRT_FUNCTION(__bswapsi2)
>> +DEFINE_COMPILERRT_THUMB_FUNCTION(__bswapsi2)
>>  #if __ARM_ARCH < 6
>>      // before armv6 does not have "rev" instruction
>>         eor     r1, r0, r0, ror #16
>>
>> Modified: compiler-rt/trunk/lib/builtins/arm/clzdi2.S
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/arm/clzdi2.S?rev=219040&r1=219039&r2=219040&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/builtins/arm/clzdi2.S (original)
>> +++ compiler-rt/trunk/lib/builtins/arm/clzdi2.S Fri Oct  3 19:18:59 2014
>> @@ -21,7 +21,7 @@
>>
>>
>>         .p2align        2
>> -DEFINE_COMPILERRT_FUNCTION(__clzdi2)
>> +DEFINE_COMPILERRT_THUMB_FUNCTION(__clzdi2)
>>  #ifdef __ARM_FEATURE_CLZ
>>  #ifdef __ARMEB__
>>         cmp     r0, 0
>>
>> Modified: compiler-rt/trunk/lib/builtins/arm/clzsi2.S
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/arm/clzsi2.S?rev=219040&r1=219039&r2=219040&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/builtins/arm/clzsi2.S (original)
>> +++ compiler-rt/trunk/lib/builtins/arm/clzsi2.S Fri Oct  3 19:18:59 2014
>> @@ -20,7 +20,7 @@
>>  #endif
>>
>>         .p2align        2
>> -DEFINE_COMPILERRT_FUNCTION(__clzsi2)
>> +DEFINE_COMPILERRT_THUMB_FUNCTION(__clzsi2)
>>  #ifdef __ARM_FEATURE_CLZ
>>         clz     r0, r0
>>         JMP(lr)
>>
>> Modified: compiler-rt/trunk/lib/builtins/arm/divmodsi4.S
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/arm/divmodsi4.S?rev=219040&r1=219039&r2=219040&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/builtins/arm/divmodsi4.S (original)
>> +++ compiler-rt/trunk/lib/builtins/arm/divmodsi4.S Fri Oct  3 19:18:59
>> 2014
>> @@ -32,7 +32,7 @@
>>  @   value is the quotient, the remainder is placed in the variable.
>>
>>         .p2align 3
>> -DEFINE_COMPILERRT_FUNCTION(__divmodsi4)
>> +DEFINE_COMPILERRT_THUMB_FUNCTION(__divmodsi4)
>>  #if __ARM_ARCH_EXT_IDIV__
>>         tst     r1, r1
>>         beq     LOCAL_LABEL(divzero)
>>
>> Modified: compiler-rt/trunk/lib/builtins/arm/divsi3.S
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/arm/divsi3.S?rev=219040&r1=219039&r2=219040&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/builtins/arm/divsi3.S (original)
>> +++ compiler-rt/trunk/lib/builtins/arm/divsi3.S Fri Oct  3 19:18:59 2014
>> @@ -33,7 +33,7 @@ DEFINE_AEABI_FUNCTION_ALIAS(__aeabi_idiv
>>  @ int __divsi3(int divident, int divisor)
>>  @   Calculate and return the quotient of the (signed) division.
>>
>> -DEFINE_COMPILERRT_FUNCTION(__divsi3)
>> +DEFINE_COMPILERRT_THUMB_FUNCTION(__divsi3)
>>  #if __ARM_ARCH_EXT_IDIV__
>>     tst     r1,r1
>>     beq     LOCAL_LABEL(divzero)
>>
>> Modified: compiler-rt/trunk/lib/builtins/arm/modsi3.S
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/arm/modsi3.S?rev=219040&r1=219039&r2=219040&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/builtins/arm/modsi3.S (original)
>> +++ compiler-rt/trunk/lib/builtins/arm/modsi3.S Fri Oct  3 19:18:59 2014
>> @@ -30,7 +30,7 @@
>>  @   Calculate and return the remainder of the (signed) division.
>>
>>         .p2align 3
>> -DEFINE_COMPILERRT_FUNCTION(__modsi3)
>> +DEFINE_COMPILERRT_THUMB_FUNCTION(__modsi3)
>>  #if __ARM_ARCH_EXT_IDIV__
>>         tst     r1, r1
>>         beq     LOCAL_LABEL(divzero)
>>
>> Modified: compiler-rt/trunk/lib/builtins/arm/sync-ops.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/arm/sync-ops.h?rev=219040&r1=219039&r2=219040&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/builtins/arm/sync-ops.h (original)
>> +++ compiler-rt/trunk/lib/builtins/arm/sync-ops.h Fri Oct  3 19:18:59 2014
>> @@ -19,7 +19,7 @@
>>          .p2align 2 ; \
>>          .thumb ; \
>>          .syntax unified ; \
>> -        DEFINE_COMPILERRT_FUNCTION(__sync_fetch_and_ ## op) \
>> +        DEFINE_COMPILERRT_THUMB_FUNCTION(__sync_fetch_and_ ## op) \
>>          dmb ; \
>>          mov r12, r0 ; \
>>          LOCAL_LABEL(tryatomic_ ## op): \
>> @@ -35,7 +35,7 @@
>>          .p2align 2 ; \
>>          .thumb ; \
>>          .syntax unified ; \
>> -        DEFINE_COMPILERRT_FUNCTION(__sync_fetch_and_ ## op) \
>> +        DEFINE_COMPILERRT_THUMB_FUNCTION(__sync_fetch_and_ ## op) \
>>          push {r4, r5, r6, lr} ; \
>>          dmb ; \
>>          mov r12, r0 ; \
>>
>> Modified: compiler-rt/trunk/lib/builtins/arm/udivmodsi4.S
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/arm/udivmodsi4.S?rev=219040&r1=219039&r2=219040&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/builtins/arm/udivmodsi4.S (original)
>> +++ compiler-rt/trunk/lib/builtins/arm/udivmodsi4.S Fri Oct  3 19:18:59
>> 2014
>> @@ -27,7 +27,7 @@
>>  @   value is the quotient, the remainder is placed in the variable.
>>
>>         .p2align 2
>> -DEFINE_COMPILERRT_FUNCTION(__udivmodsi4)
>> +DEFINE_COMPILERRT_THUMB_FUNCTION(__udivmodsi4)
>>  #if __ARM_ARCH_EXT_IDIV__
>>         tst     r1, r1
>>         beq     LOCAL_LABEL(divby0)
>>
>> Modified: compiler-rt/trunk/lib/builtins/arm/udivsi3.S
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/arm/udivsi3.S?rev=219040&r1=219039&r2=219040&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/builtins/arm/udivsi3.S (original)
>> +++ compiler-rt/trunk/lib/builtins/arm/udivsi3.S Fri Oct  3 19:18:59 2014
>> @@ -27,7 +27,7 @@ DEFINE_AEABI_FUNCTION_ALIAS(__aeabi_uidi
>>  @ unsigned int __udivsi3(unsigned int divident, unsigned int divisor)
>>  @   Calculate and return the quotient of the (unsigned) division.
>>
>> -DEFINE_COMPILERRT_FUNCTION(__udivsi3)
>> +DEFINE_COMPILERRT_THUMB_FUNCTION(__udivsi3)
>>  #if __ARM_ARCH_EXT_IDIV__
>>         tst     r1, r1
>>         beq     LOCAL_LABEL(divby0)
>>
>> Modified: compiler-rt/trunk/lib/builtins/arm/umodsi3.S
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/arm/umodsi3.S?rev=219040&r1=219039&r2=219040&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/builtins/arm/umodsi3.S (original)
>> +++ compiler-rt/trunk/lib/builtins/arm/umodsi3.S Fri Oct  3 19:18:59 2014
>> @@ -24,7 +24,7 @@
>>  @   Calculate and return the remainder of the (unsigned) division.
>>
>>         .p2align 2
>> -DEFINE_COMPILERRT_FUNCTION(__umodsi3)
>> +DEFINE_COMPILERRT_THUMB_FUNCTION(__umodsi3)
>>  #if __ARM_ARCH_EXT_IDIV__
>>         tst     r1, r1
>>         beq     LOCAL_LABEL(divby0)
>>
>> Modified: compiler-rt/trunk/lib/builtins/assembly.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/assembly.h?rev=219040&r1=219039&r2=219040&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/builtins/assembly.h (original)
>> +++ compiler-rt/trunk/lib/builtins/assembly.h Fri Oct  3 19:18:59 2014
>> @@ -28,6 +28,10 @@
>>  // tell linker it can break up file at label boundaries
>>  #define FILE_LEVEL_DIRECTIVE .subsections_via_symbols
>>  #define SYMBOL_IS_FUNC(name)
>> +// rdar://problem/18523605
>> +#if __ARM_ARCH_ISA_THUMB == 2
>> +#define THUMB_FUNC .thumb_func
>> +#endif
>>  #elif defined(__ELF__)
>>  #define HIDDEN(name) .hidden name
>>  #define LOCAL_LABEL(name) .L_##name
>> @@ -97,6 +101,14 @@
>>    SYMBOL_IS_FUNC(SYMBOL_NAME(name)) SEPARATOR
>>       \
>>    DECLARE_SYMBOL_VISIBILITY(name)
>>       \
>>    SYMBOL_NAME(name):
>> +
>> +#define DEFINE_COMPILERRT_THUMB_FUNCTION(name)
>>        \
>> +  FILE_LEVEL_DIRECTIVE SEPARATOR
>>        \
>> +  .globl SYMBOL_NAME(name) SEPARATOR
>>        \
>> +  THUMB_FUNC
>>        \
>> +  SYMBOL_IS_FUNC(SYMBOL_NAME(name)) SEPARATOR
>>       \
>> +  DECLARE_SYMBOL_VISIBILITY(name)
>>       \
>> +  SYMBOL_NAME(name):
>>
>>  #define DEFINE_COMPILERRT_PRIVATE_FUNCTION(name)
>>        \
>>    FILE_LEVEL_DIRECTIVE SEPARATOR
>>        \
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
>
> --
> Saleem Abdulrasool
> compnerd (at) compnerd (dot) org
>
>
>


-- 
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141006/029528a5/attachment.html>


More information about the llvm-commits mailing list