Message ID | 11626389.827m83Rx9E@e108577-lin |
---|---|
State | New |
Headers | show |
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
diff --git a/gcc/config/arm/elf.h b/gcc/config/arm/elf.h index 77f30554d5286bd83aeab0c8dc308cfd44e732dc..246de5492665ba2a0292736a9c53fbaaef184d72 100644 --- a/gcc/config/arm/elf.h +++ b/gcc/config/arm/elf.h @@ -148,8 +148,9 @@ while (0) /* Horrible hack: We want to prevent some libgcc routines being included - for some multilibs. */ -#ifndef __ARM_ARCH_6M__ + for some multilibs. The condition should match the one in + libgcc/config/arm/lib1funcs.S. */ +#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 #undef L_fixdfsi #undef L_fixunsdfsi #undef L_truncdfsf2 diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 04ca17656f2f26dda710e8a0f9ca77dd963ab39b..38151375c29cd007f1cc34ead3aa495606224061 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -3320,10 +3320,8 @@ proc check_effective_target_arm_cortex_m { } { return 0 } return [check_no_compiler_messages arm_cortex_m assembly { - #if !defined(__ARM_ARCH_7M__) \ - && !defined (__ARM_ARCH_7EM__) \ - && !defined (__ARM_ARCH_6M__) - #error !__ARM_ARCH_7M__ && !__ARM_ARCH_7EM__ && !__ARM_ARCH_6M__ + #if defined(__ARM_ARCH_ISA_ARM) + #error __ARM_ARCH_ISA_ARM is defined #endif int i; } "-mthumb"] diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S index 375a5135110895faa44267ebee045fd315515027..7268deb37f2c4ce4e43eeac06c418235071ebcd2 100644 --- a/libgcc/config/arm/lib1funcs.S +++ b/libgcc/config/arm/lib1funcs.S @@ -124,7 +124,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see && !defined(__thumb2__) \ && (!defined(__THUMB_INTERWORK__) \ || defined (__OPTIMIZE_SIZE__) \ - || defined(__ARM_ARCH_6M__))) + || !__ARM_ARCH_ISA_ARM)) # define __prefer_thumb__ #endif @@ -305,7 +305,7 @@ LSYM(Lend_fde): #ifdef __ARM_EABI__ .macro THUMB_LDIV0 name signed -#if defined(__ARM_ARCH_6M__) +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 .ifc \signed, unsigned cmp r0, #0 beq 1f @@ -478,7 +478,7 @@ _L__\name: #else /* !(__INTERWORKING_STUBS__ || __thumb2__) */ -#ifdef __ARM_ARCH_6M__ +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 #define EQUIV .thumb_set #else .macro ARM_FUNC_START name sp_section= @@ -510,7 +510,7 @@ SYM (__\name): #endif .endm -#ifndef __ARM_ARCH_6M__ +#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 .macro ARM_FUNC_ALIAS new old .globl SYM (__\new) EQUIV SYM (__\new), SYM (__\old) @@ -1054,7 +1054,7 @@ ARM_FUNC_START aeabi_uidivmod /* ------------------------------------------------------------------------ */ #ifdef L_umodsi3 -#ifdef __ARM_ARCH_EXT_IDIV__ +#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1 ARM_FUNC_START umodsi3 @@ -1240,7 +1240,7 @@ ARM_FUNC_START aeabi_idivmod /* ------------------------------------------------------------------------ */ #ifdef L_modsi3 -#if defined(__ARM_ARCH_EXT_IDIV__) +#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1 ARM_FUNC_START modsi3 @@ -1508,14 +1508,8 @@ LSYM(Lover12): #endif /* __symbian__ */ -#if ((__ARM_ARCH__ > 5) && !defined(__ARM_ARCH_6M__)) \ - || defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \ - || defined(__ARM_ARCH_5TEJ__) -#define HAVE_ARM_CLZ 1 -#endif - #ifdef L_clzsi2 -#if defined(__ARM_ARCH_6M__) +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 FUNC_START clzsi2 mov r1, #28 mov r3, #1 @@ -1544,7 +1538,7 @@ FUNC_START clzsi2 FUNC_END clzsi2 #else ARM_FUNC_START clzsi2 -# if defined(HAVE_ARM_CLZ) +# if defined(__ARM_FEATURE_CLZ) clz r0, r0 RET # else @@ -1568,15 +1562,15 @@ ARM_FUNC_START clzsi2 .align 2 1: .byte 4, 3, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0 -# endif /* !HAVE_ARM_CLZ */ +# endif /* !__ARM_FEATURE_CLZ */ FUNC_END clzsi2 #endif #endif /* L_clzsi2 */ #ifdef L_clzdi2 -#if !defined(HAVE_ARM_CLZ) +#if !defined(__ARM_FEATURE_CLZ) -# if defined(__ARM_ARCH_6M__) +# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 FUNC_START clzdi2 push {r4, lr} # else @@ -1601,14 +1595,14 @@ ARM_FUNC_START clzdi2 bl __clzsi2 # endif 2: -# if defined(__ARM_ARCH_6M__) +# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 pop {r4, pc} # else RETLDM r4 # endif FUNC_END clzdi2 -#else /* HAVE_ARM_CLZ */ +#else /* __ARM_FEATURE_CLZ */ ARM_FUNC_START clzdi2 cmp xxh, #0 @@ -1623,7 +1617,7 @@ ARM_FUNC_START clzdi2 #endif /* L_clzdi2 */ #ifdef L_ctzsi2 -#if defined(__ARM_ARCH_6M__) +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 FUNC_START ctzsi2 neg r1, r0 and r0, r0, r1 @@ -1656,7 +1650,7 @@ FUNC_START ctzsi2 ARM_FUNC_START ctzsi2 rsb r1, r0, #0 and r0, r0, r1 -# if defined(HAVE_ARM_CLZ) +# if defined(__ARM_FEATURE_CLZ) clz r0, r0 rsb r0, r0, #31 RET @@ -1681,7 +1675,7 @@ ARM_FUNC_START ctzsi2 .align 2 1: .byte 27, 28, 29, 29, 30, 30, 30, 30, 31, 31, 31, 31, 31, 31, 31, 31 -# endif /* !HAVE_ARM_CLZ */ +# endif /* !__ARM_FEATURE_CLZ */ FUNC_END ctzsi2 #endif #endif /* L_clzsi2 */ @@ -1738,7 +1732,7 @@ ARM_FUNC_START ctzsi2 /* Don't bother with the old interworking routines for Thumb-2. */ /* ??? Maybe only omit these on "m" variants. */ -#if !defined(__thumb2__) && !defined(__ARM_ARCH_6M__) +#if !defined(__thumb2__) && __ARM_ARCH_ISA_ARM #if defined L_interwork_call_via_rX @@ -1983,11 +1977,12 @@ LSYM(Lchange_\register): .endm #ifndef __symbian__ -#ifndef __ARM_ARCH_6M__ +/* The condition here must match the one in gcc/config/arm/elf.h. */ +#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 #include "ieee754-df.S" #include "ieee754-sf.S" #include "bpabi.S" -#else /* __ARM_ARCH_6M__ */ +#else /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */ #include "bpabi-v6m.S" -#endif /* __ARM_ARCH_6M__ */ +#endif /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */ #endif /* !__symbian__ */ diff --git a/libgcc/config/arm/libunwind.S b/libgcc/config/arm/libunwind.S index a68b10ddce93d230c504f3747dcad3832d9f753c..3d7e70181fa80fe53a4903c96bb0f90480feee21 100644 --- a/libgcc/config/arm/libunwind.S +++ b/libgcc/config/arm/libunwind.S @@ -58,7 +58,7 @@ #endif #endif -#ifdef __ARM_ARCH_6M__ +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 /* r0 points to a 16-word block. Upload these values to the actual core state. */ @@ -169,7 +169,7 @@ FUNC_START gnu_Unwind_Save_WMMXC UNPREFIX \name .endm -#else /* !__ARM_ARCH_6M__ */ +#else /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */ /* r0 points to a 16-word block. Upload these values to the actual core state. */ @@ -351,7 +351,7 @@ ARM_FUNC_START gnu_Unwind_Save_WMMXC UNPREFIX \name .endm -#endif /* !__ARM_ARCH_6M__ */ +#endif /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */ UNWIND_WRAPPER _Unwind_RaiseException 1 UNWIND_WRAPPER _Unwind_Resume 1 diff --git a/libgcc/config/arm/t-softfp b/libgcc/config/arm/t-softfp index 4ede438baf6a297737e52db00395f6c3a359f681..554ec9bc47b04445e79e84b1f957bf88680c08d1 100644 --- a/libgcc/config/arm/t-softfp +++ b/libgcc/config/arm/t-softfp @@ -1,2 +1,2 @@ -softfp_wrap_start := '\#ifdef __ARM_ARCH_6M__' +softfp_wrap_start := '\#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1' softfp_wrap_end := '\#endif'