[PATCH, libgcc/ARM 1/6, ping1] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
Kyrill Tkachov
kyrylo.tkachov@foss.arm.com
Wed May 25 15:30:00 GMT 2016
Hi Thomas,
On 25/05/16 14:22, Thomas Preudhomme wrote:
> Hi Kyrill,
>
> Please find an updated version below. Note that I also:
>
> * removed the change to bpabi-v6m.S because that actually accurately describe
> the implementation (using instructions from ARMv6-M) and does not suggest it
> is not compatible with other architecture (it does not say ARMv6-M only)
> * kept the TARGET_ARM_V*M unchanged, this is now dealt in a separate patch
>
>
> ChangeLog entries are now as follow:
>
>
> *** gcc/ChangeLog ***
>
> 2016-05-23 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM to
> decide whether to prevent some libgcc routines being included for some
> multilibs rather than __ARM_ARCH_6M__ and add comment to indicate the
> link between this condition and the one in
> libgcc/config/arm/lib1func.S.
>
>
> *** gcc/testsuite/ChangeLog ***
>
> 2015-11-10 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> * lib/target-supports.exp (check_effective_target_arm_cortex_m): Use
> __ARM_ARCH_ISA_ARM to test for Cortex-M devices.
>
>
> *** libgcc/ChangeLog ***
>
> 2016-05-23 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> * config/arm/lib1funcs.S (__prefer_thumb__): Define among other cases
> for all Thumb-1 only targets.
> (__only_thumb1__): Define for all Thumb-1 only targets.
> (THUMB_LDIV0): Test for __only_thumb1__ rather than __ARM_ARCH_6M__.
> (EQUIV): Likewise.
> (ARM_FUNC_ALIAS): Likewise.
> (umodsi3): Add check to __only_thumb1__ to guard the idiv version.
> (modsi3): Likewise.
> (HAVE_ARM_CLZ): Remove block defining it.
> (clzsi2): Test for __only_thumb1__ rather than __ARM_ARCH_6M__ and
> check __ARM_FEATURE_CLZ instead of HAVE_ARM_CLZ.
> (clzdi2): Likewise.
> (ctzsi2): Likewise.
> (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather than
> __ARM_ARCH_6M__ in guard for checking whether it is defined.
> (final includes): Test for __only_thumb1__ rather than
> __ARM_ARCH_6M__ and add comment to indicate the connection between
> this condition and the one in gcc/config/arm/elf.h.
> * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
> __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
> * config/arm/t-softfp: Likewise.
>
>
> Updated patch is attached to this mail.
Thanks, the gcc/ changes look ok to me.
The libgcc/ changes look sensible to me too, but I don't know libgcc
very well so there could be something I'm missing.
So please wait for an ok from an arm maintainer on this.
Kyrill
> Best regards,
>
> Thomas
>
> On Thursday 19 May 2016 18:01:16 Kyrill Tkachov wrote:
>> On 19/05/16 17:55, Thomas Preudhomme wrote:
>>> On Thursday 19 May 2016 17:42:26 Kyrill Tkachov wrote:
>>>> Hi Thomas,
>>>>
>>>> I'm not very familiar with the libgcc machinery, but I have a comment on
>>>> an
>>>> arm.h hunk inline.
>>>>
>>>> On 17/05/16 10:58, Thomas Preudhomme wrote:
>>>>> Ping?
>>>>>
>>>>> *** gcc/ChangeLog ***
>>>>>
>>>>> 2015-11-13 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>
>>>>> * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and
>>>>> __ARM_ARCH_ISA_ARM to
>>>>> decide whether to prevent some libgcc routines being included
>>>>> for
>>>>> some
>>>>> multilibs rather than __ARM_ARCH_6M__ and add comment to
>>>>> indicate
>>>>> the
>>>>> link between this condition and the one in
>>>>> libgcc/config/arm/lib1func.S.
>>>>> * config/arm/arm.h (TARGET_ARM_V6M): Add check to
>>>>> TARGET_ARM_ARCH.
>>>>> (TARGET_ARM_V7M): Likewise.
>>>>>
>>>>> *** gcc/testsuite/ChangeLog ***
>>>>>
>>>>> 2015-11-10 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>
>>>>> * lib/target-supports.exp
>>>>> (check_effective_target_arm_cortex_m):
>>>>> Use
>>>>> __ARM_ARCH_ISA_ARM to test for Cortex-M devices.
>>>>>
>>>>> *** libgcc/ChangeLog ***
>>>>>
>>>>> 2015-12-17 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>
>>>>> * config/arm/bpabi-v6m.S: Fix header comment to mention
>>>>> Thumb-1
>>>>> rather
>>>>> than ARMv6-M.
>>>>> * config/arm/lib1funcs.S (__prefer_thumb__): Define among
>>>>> other
>>>>> cases
>>>>> for all Thumb-1 only targets.
>>>>> (__only_thumb1__): Define for all Thumb-1 only targets.
>>>>> (THUMB_LDIV0): Test for __only_thumb1__ rather than
>>>>> __ARM_ARCH_6M__.
>>>>> (EQUIV): Likewise.
>>>>> (ARM_FUNC_ALIAS): Likewise.
>>>>> (umodsi3): Add check to __only_thumb1__ to guard the idiv
>>>>> version.
>>>>> (modsi3): Likewise.
>>>>> (HAVE_ARM_CLZ): Remove block defining it.
>>>>> (clzsi2): Test for __only_thumb1__ rather than __ARM_ARCH_6M__
>>>>> and
>>>>> check __ARM_FEATURE_CLZ instead of HAVE_ARM_CLZ.
>>>>> (clzdi2): Likewise.
>>>>> (ctzsi2): Likewise.
>>>>> (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather
>>>>> than
>>>>> __ARM_ARCH_6M__ in guard for checking whether it is defined.
>>>>> (final includes): Test for __only_thumb1__ rather than
>>>>> __ARM_ARCH_6M__ and add comment to indicate the connection
>>>>> between
>>>>> this condition and the one in gcc/config/arm/elf.h.
>>>>> * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
>>>>> __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
>>>>> * config/arm/t-softfp: Likewise.
>>>>>
>>>>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
>>>>> index
>>>>> 5b1a03080d0a00fc1ef6934f6bce552e65230640..7eb11d920944c693700d80bb3fb3f9
>>>>> fe
>>>>> 66611c19 100644
>>>>> --- a/gcc/config/arm/arm.h
>>>>> +++ b/gcc/config/arm/arm.h
>>>>> @@ -2188,8 +2188,10 @@ extern int making_const_table;
>>>>>
>>>>> #define TARGET_ARM_ARCH \
>>>>>
>>>>> (arm_base_arch) \
>>>>>
>>>>> -#define TARGET_ARM_V6M (!arm_arch_notm && !arm_arch_thumb2)
>>>>> -#define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2)
>>>>> +#define TARGET_ARM_V6M (TARGET_ARM_ARCH == BASE_ARCH_6M &&
>>>>> !arm_arch_notm
>>>>> \ + && !arm_arch_thumb2)
>>>>> +#define TARGET_ARM_V7M (TARGET_ARM_ARCH == BASE_ARCH_7M &&
>>>>> !arm_arch_notm
>>>>> \ + && arm_arch_thumb2)
>>>> I think now that you're checking TARGET_ARM_ARCH you don't need
>>>> the "!arm_arch_notm && !arm_arch_thumb2" && "!arm_arch_notm &&
>>>> arm_arch_thumb2".
>>> % git --no-pager grep TARGET_ARM_V.M config/arm
>>> config/arm/arm.h:#define TARGET_ARM_V6M (!arm_arch_notm &&
>>> !arm_arch_thumb2) config/arm/arm.h:#define TARGET_ARM_V7M (!arm_arch_notm
>>> && arm_arch_thumb2)
>>>
>>> What about just removing those? I kept them because I wasn't sure of their
>>> purpose but I think we should just remove them.
>> That's fine with me.
>> Then you'd also want to remove the TARGET_ARM_V8M definition from your
>> second patch that I flagged up in that review.
>>
>> Kyrill
>>
>>> Best regards,
>>>
>>> Thomas
More information about the Gcc-patches
mailing list