[compiler-rt] r314284 - [Builtins] ARM: Fix msr assembly instruction use for Thumb2.

Wed Sep 27 12:13:57 PDT 2017

I have reverted all 3 relevant CLs.
Tim, please review the patch so that I can land them again. Without these
CLs, there is a builtin linkage mess where function declare themselves as
thumb but are assembled in arm encoding.
( PR 34715).

Thanks,
Manoj

On Wed, Sep 27, 2017 at 12:00 PM, Manoj Gupta <manojgupta at google.com> wrote:

> Ok, Let me do a revert for now.
>
> On Wed, Sep 27, 2017 at 11:58 AM, Juergen Ributzka <juergen at ributzka.de>
> wrote:
>
>> At this point our CI has been broken for over 8h, so I would prefer to
>> revert the change until Tim had a time to review the patch.
>>
>> —Juergen
>>
>>
>> On Sep 27, 2017, at 11:49 AM, Juergen Ributzka via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>
>>
>> On Sep 27, 2017, at 11:34 AM, Manoj Gupta <manojgupta at google.com> wrote:
>>
>> The following confirms that armv7k target wants thumb as default:
>>
>> // No -mthumb in argument
>> $ clang -target armv7k-watchos   -mcpu=cortex-a7 -x c -E -dM /dev/null -o
>> -|grep -i thumb
>> #define __ARM_ARCH_ISA_THUMB 2
>> #define __THUMBEL__ 1
>> #define __THUMB_INTERWORK__ 1
>> #define __thumb2__ 1
>> #define __thumb__ 1
>>
>> // Add -marm
>>  $ clang -target armv7k-watchos -marm  -mcpu=cortex-a7 -x c -E -dM
>> /dev/null -o -|grep -i thumb
>> #define __ARM_ARCH_ISA_THUMB 2
>> #define __THUMB_INTERWORK__ 1
>>
>>
>> I assume that following patch to lib/builtins/assembly.h should fix it
>> But I should probably add an apple check as well to 7K. What should that be?
>>
>>
>> Sorry, I don’t know either. I am not familiar with this code, so I defer
>> to Tim who is the expert.
>>
>>
>> diff --git a/lib/builtins/assembly.h b/lib/builtins/assembly.h
>> index 3f5e59b25..206c9b29c 100644
>> --- a/lib/builtins/assembly.h
>> +++ b/lib/builtins/assembly.h
>> @@ -69,12 +69,20 @@
>>
>>  #if defined(__arm__)
>>
>> +#ifdef __ARM_ARCH_7K__
>> +
>> +#define DEFINE_CODE_STATE .arm SEPARATOR
>> +#define DECLARE_FUNC_ENCODING
>> +#define IT(cond)
>> +#define ITT(cond)
>> +#define ITE(cond)
>> +
>>  /*
>>   * Determine actual [ARM][THUMB[1][2]] ISA using compiler predefined
>> macros:
>>   * - for '-mthumb -march=armv6' compiler defines '__thumb__'
>>   * - for '-mthumb -march=armv7' compiler defines '__thumb__' and
>> '__thumb2__'
>>   */
>> -#if defined(__thumb2__) || defined(__thumb__)
>> +#elif defined(__thumb2__) || defined(__thumb__)
>>  #define DEFINE_CODE_STATE .thumb SEPARATOR
>>  #define DECLARE_FUNC_ENCODING    .thumb_func SEPARATOR
>>  #if defined(__thumb2__)
>>
>>
>> On Wed, Sep 27, 2017 at 11:32 AM, Juergen Ributzka <juergen at apple.com> w
>> rote:
>>
>>> [+Tim]
>>>
>>>
>>> On Sep 27, 2017, at 11:14 AM, Manoj Gupta <manojgupta at google.com> wrote:
>>>
>>> I don't know much about armv7k. But adding -marm to the command line
>>> fixes the error.
>>> clang    -O2 -g -DNDEBUG     -fPIC -O3 -fvisibility=hidden
>>> -DVISIBILITY_HIDDEN -Wall  -arch armv7k -MMD -MT aeabi_cdcmp.S.o -MF
>>> aeabi_cdcmp.S.o.d -o aeabi_cdcmp.S.o -c aeabi_cdcmp.S -target
>>> armv7k-watchos // This fails
>>> clang    -O2 -g -DNDEBUG     -fPIC -O3 -fvisibility=hidden
>>> -DVISIBILITY_HIDDEN -Wall  -arch armv7k -MMD -MT aeabi_cdcmp.S.o -MF
>>> aeabi_cdcmp.S.o.d -o aeabi_cdcmp.S.o -c aeabi_cdcmp.S -target
>>> armv7k-watchos -marm // This works
>>>
>>> This makes me think that the using armv7k target defaults to thumb but
>>> it wants compilation to be done in ARM mode for the builtins?
>>>
>>> Thanks,
>>> Manoj
>>>
>>>
>>>
>>> On Wed, Sep 27, 2017 at 10:46 AM, Juergen Ributzka <juergen at ributzka.de>
>>>  wrote:
>>>
>>>> I think this broke Green Dragon. Could you please take a look?
>>>>
>>>> http://green.lab.llvm.org/green/job/clang-stage1-configure-R
>>>> A_build/39186/consoleFull#-187600398149ba4694-19c4-4d7e-bec5
>>>> -911270d8a58c
>>>>
>>>> Thanks
>>>>
>>>> Cheers,
>>>> Juergen
>>>>
>>>> On Wed, Sep 27, 2017 at 2:29 AM, Manoj Gupta via llvm-commits <
>>>> llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>> Author: manojgupta
>>>>> Date: Wed Sep 27 02:29:57 2017
>>>>> New Revision: 314284
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=314284&view=rev
>>>>> Log:
>>>>> [Builtins] ARM: Fix msr assembly instruction use for Thumb2.
>>>>>
>>>>> Summary:
>>>>> MSR instruction in Thumb2 does not support immediate operand.
>>>>> Fix this by moving the condition for V7-M to Thumb2 since V7-M support
>>>>> Thumb2 only. With this change, aeabi_cfcmp.s and aeabi_cdcmp.S files
>>>>> can
>>>>> be assembled in Thumb2 mode. (This is split out from the review
>>>>> D38227).
>>>>>
>>>>> Reviewers: compnerd, peter.smith, srhines, weimingz, rengolin,
>>>>> kristof.beyls
>>>>>
>>>>> Reviewed By: compnerd
>>>>>
>>>>> Subscribers: aemerson, javed.absar, llvm-commits
>>>>>
>>>>> Differential Revision: https://reviews.llvm.org/D38268
>>>>>
>>>>> Modified:
>>>>>     compiler-rt/trunk/lib/builtins/arm/aeabi_cdcmp.S
>>>>>     compiler-rt/trunk/lib/builtins/arm/aeabi_cfcmp.S
>>>>>
>>>>> Modified: compiler-rt/trunk/lib/builtins/arm/aeabi_cdcmp.S
>>>>> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/
>>>>> lib/builtins/arm/aeabi_cdcmp.S?rev=314284&r1=314283&r2=
>>>>> 314284&view=diff
>>>>> ============================================================
>>>>> ==================
>>>>> --- compiler-rt/trunk/lib/builtins/arm/aeabi_cdcmp.S (original)
>>>>> +++ compiler-rt/trunk/lib/builtins/arm/aeabi_cdcmp.S Wed Sep 27
>>>>> 02:29:57 2017
>>>>> @@ -48,7 +48,7 @@ DEFINE_COMPILERRT_FUNCTION(__aeabi_cdcmp
>>>>>          // NaN has been ruled out, so __aeabi_cdcmple can't trap
>>>>>          bne __aeabi_cdcmple
>>>>>
>>>>> -#if defined(__ARM_ARCH_7M__) || defined(__ARM_ARCH_7EM__)
>>>>> +#if defined(USE_THUMB_2)
>>>>>          mov ip, #APSR_C
>>>>>          msr APSR_nzcvq, ip
>>>>>  #else
>>>>>
>>>>> Modified: compiler-rt/trunk/lib/builtins/arm/aeabi_cfcmp.S
>>>>> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/
>>>>> lib/builtins/arm/aeabi_cfcmp.S?rev=314284&r1=314283&r2=
>>>>> 314284&view=diff
>>>>> ============================================================
>>>>> ==================
>>>>> --- compiler-rt/trunk/lib/builtins/arm/aeabi_cfcmp.S (original)
>>>>> +++ compiler-rt/trunk/lib/builtins/arm/aeabi_cfcmp.S Wed Sep 27
>>>>> 02:29:57 2017
>>>>> @@ -48,7 +48,7 @@ DEFINE_COMPILERRT_FUNCTION(__aeabi_cfcmp
>>>>>          // NaN has been ruled out, so __aeabi_cfcmple can't trap
>>>>>          bne __aeabi_cfcmple
>>>>>
>>>>> -#if defined(__ARM_ARCH_7M__) || defined(__ARM_ARCH_7EM__)
>>>>> +#if defined(USE_THUMB_2)
>>>>>          mov ip, #APSR_C
>>>>>          msr APSR_nzcvq, ip
>>>>>  #else
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>
>>>>
>>>
>>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170927/13faeca0/attachment.html>