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

Wed Sep 27 12:38:59 PDT 2017

Thank you Manoj

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

> 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/l
>>>>>> ib/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/l
>>>>>> ib/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/d4819379/attachment.html>