[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