[compiler-rt] r219040 - Fix the armv7 thumb builtins on darwin
Steven Wu
stevenwu at apple.com
Mon Oct 6 18:24:49 PDT 2014
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.
> 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 <mailto: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 <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 <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 <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 <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 <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 <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 <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 <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 <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 <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 <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 <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 <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 <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>
>
>
> --
> Saleem Abdulrasool
> compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141006/26eb44ac/attachment.html>
More information about the llvm-commits
mailing list