[PATCH] D38227: [Builtins] ARM: Fix assembling files in thumb mode.

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

Apologies for being a pain, could you split the changes into their own review, even if they depend on each other? It will be easier to review, and to commit each one separately.



================
Comment at: lib/builtins/arm/aeabi_cdcmp.S:54-56
+#elif defined(USE_THUMB_2)
+        mov ip, #APSR_C
+        msr CPSR_f, ip
----------------
manojgupta wrote:
> Please review this change and the one below since I am not much of an ARM assembly expert.
I think that this would be better split out into a separate patch, potentially dependent on this one. 

I think that you don't need an extra case for Thumb2. __ARM_ARCH_7M__ and __ARM_ARCH_7EM__ only support Thumb 2 so I think all you need to do is replace that condition with USE_THUMB2.

Apologies I've not got an assembler at hand to check.


================
Comment at: lib/builtins/arm/aeabi_memset.S:27
 
+        .p2align 2
 DEFINE_COMPILERRT_FUNCTION(__aeabi_memclr)
----------------
manojgupta wrote:
> This was the only function I found without a .p2align. Please let me know if this change is not required.
Arm functions need to be 4-byte aligned, Thumb functions 2-byte aligned. We might be able to move this to be part of DEFINE_COMPILERRT_FUNCTION so that it can be set to 2 or 4. Having it set to 4 as you suggest is probably good enough for now.


https://reviews.llvm.org/D38227