Message ID | 4707964.lx3ShB1mLb@e108577-lin |
---|---|
State | New |
Headers | show |
[Fixed subject to reflect patch] Ping? Best regards, Thomas On Monday 27 June 2016 17:51:34 Thomas Preudhomme wrote: > Hi Ramana, > > On Wednesday 01 June 2016 10:00:52 Ramana Radhakrishnan wrote: > > From here down to .... > > > > > -#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) > > > > here should be it's own little patchlet and can go in separately. > > The patch in attachment changes the CLZ availability check in libgcc to test > ISA supported and architecture version rather than encode a specific list > of architectures. __ARM_FEATURE_CLZ is not used because its value depends > on what mode the user is targeting but only the architecture support > matters in this case. Indeed, the code using CLZ is written in assembler > and uses mnemonics available both in ARM and Thumb mode so only CLZ > availability in one of the mode matters. > > This change was split out from [PATCH, GCC, ARM 1/7] Fix Thumb-1 only == > ARMv6-M & Thumb-2 only == ARMv7-M assumptions. > > ChangeLog entry is as follows: > > *** libgcc/ChangeLog *** > > 2016-06-16 Thomas Preud'homme <thomas.preudhomme@arm.com> > > * config/arm/lib1funcs.S (HAVE_ARM_CLZ): Define for ARMv6* or later > and ARMv5t* rather than for a fixed list of architectures. > > Looking for code generation change accross a number of combinations of ISAs > (ARM/Thumb), optimization levels (Os/O2), and architectures (armv4, armv4t, > armv5, armv5t, armv5te, armv6, armv6j, armv6k, armv6s-m, armv6kz, armv6t2, > armv6z, armv6zk, armv7, armv7-a, armv7e-m, armv7-m, armv7-r, armv7ve, > armv8-a, armv8-a+crc, iwmmxt and iwmmxt2) shows that only ARMv5T is > impacted (uses CLZ now). This is expected because currently HAVE_ARM_CLZ is > not defined for this architecture while the ARMv7-a/ARMv7-R Architecture > Reference Manual [1] states that all ARMv5T* architectures have CLZ. ARMv5E > should also be impacted (not using CLZ anymore) but testing it is difficult > since current binutils does not support ARMv5E. > > [1] Document ARM DDI0406C in http://infocenter.arm.com > > Best regards, > > Thomas
On Mon, Jun 27, 2016 at 5:51 PM, Thomas Preudhomme <thomas.preudhomme@foss.arm.com> wrote: > Hi Ramana, > > On Wednesday 01 June 2016 10:00:52 Ramana Radhakrishnan wrote: >> >> From here down to .... >> >> > -#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) >> >> here should be it's own little patchlet and can go in separately. > > The patch in attachment changes the CLZ availability check in libgcc to test > ISA supported and architecture version rather than encode a specific list of > architectures. __ARM_FEATURE_CLZ is not used because its value depends on what > mode the user is targeting but only the architecture support matters in this > case. Indeed, the code using CLZ is written in assembler and uses mnemonics > available both in ARM and Thumb mode so only CLZ availability in one of the > mode matters. > > This change was split out from [PATCH, GCC, ARM 1/7] Fix Thumb-1 only == > ARMv6-M & Thumb-2 only == ARMv7-M assumptions. > > ChangeLog entry is as follows: > > *** libgcc/ChangeLog *** > > 2016-06-16 Thomas Preud'homme <thomas.preudhomme@arm.com> > > * config/arm/lib1funcs.S (HAVE_ARM_CLZ): Define for ARMv6* or later > and ARMv5t* rather than for a fixed list of architectures. > > Looking for code generation change accross a number of combinations of ISAs > (ARM/Thumb), optimization levels (Os/O2), and architectures (armv4, armv4t, > armv5, armv5t, armv5te, armv6, armv6j, armv6k, armv6s-m, armv6kz, armv6t2, > armv6z, armv6zk, armv7, armv7-a, armv7e-m, armv7-m, armv7-r, armv7ve, armv8-a, > armv8-a+crc, iwmmxt and iwmmxt2) shows that only ARMv5T is impacted (uses CLZ > now). This is expected because currently HAVE_ARM_CLZ is not defined for this > architecture while the ARMv7-a/ARMv7-R Architecture Reference Manual [1] > states that all ARMv5T* architectures have CLZ. ARMv5E should also be impacted > (not using CLZ anymore) but testing it is difficult since current binutils does > not support ARMv5E. > > [1] Document ARM DDI0406C in http://infocenter.arm.com > > Best regards, > > Thomas OK. Ramana
diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S index 951dcda1c3bf7f323423a3e2813bdf0501653016..c4f061f8196d243159903cac4eb0291d1bf0b1ad 100644 --- a/libgcc/config/arm/lib1funcs.S +++ b/libgcc/config/arm/lib1funcs.S @@ -1512,9 +1512,10 @@ LSYM(Lover12): #endif /* __symbian__ */ -#if ((__ARM_ARCH__ > 5) && !defined(__ARM_ARCH_6M__)) \ - || defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \ - || defined(__ARM_ARCH_5TEJ__) +#if (__ARM_ARCH_ISA_THUMB == 2 \ + || (__ARM_ARCH_ISA_ARM \ + && (__ARM_ARCH__ > 5 \ + || (__ARM_ARCH__ == 5 && __ARM_ARCH_ISA_THUMB)))) #define HAVE_ARM_CLZ 1 #endif