This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On 21/06/18 07:59, Christophe Lyon wrote:
On Tue, 19 Jun 2018 at 10:50, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote:Hi Christophe, On 17/06/18 21:23, Christophe Lyon wrote:On Fri, 15 Jun 2018 at 17:22, Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote:On 15/06/18 15:30, Christophe Lyon wrote:Hello, As suggested in [1], the attached patch removes all definitions and uses of __ARM_ARCH__ and uses __ARM_ARCH instead. The later is indeed defined by the preprocessor to the appropriate value. I ran make check on arm-none-eabi (with A-profile multilib), arm-none-linux-gnueabi, arm-none-linux-gnueabihf (with cortex-a9, a15, a5, a57 and armtdmi as --with-cpu), armeb-none-linux-gnueabihf and armv8l-linux-gnueabihf, and noticed no regression. OK for trunk? Thanks, Christophe [1] https://gcc.gnu.org/ml/gcc-patches/2018-06/msg00445.html ARM_ARCH.chlog.txt libatomic/ChangeLog: 2018-06-15 Christophe Lyon <christophe.lyon@linaro.org> * config/arm/arm-config.h (__ARM_ARCH__): Remove definitions, use __ARM_ARCH instead. libgcc/ChangeLog: 2018-06-15 Christophe Lyon <christophe.lyon@linaro.org> * config/arm/lib1funcs.S (__ARM_ARCH__): Remove definitions, use __ARM_ARCH instead. * config/arm/ieee754-df.S: Use __ARM_ARCH instead of __ARM_ARCH__. * config/arm/ieee754-sf.S: Likewise. * config/arm/libunwind.S: Likewise. ARM_ARCH.patch.txtThanks, this is a useful start. We can, however, go further. ACLE defines a number of 'feature' pre-defines and we can use those to void direct tests of the architecture version directly. For example, __ARM_FEATURE_LDREX could directly replace having to calculate HAVE_STREX and HAVE_STREXBHD.Hi, Here is an updated patch using __ARM_FEATURE_LDREX. I didn't find other opportunities to use ACLE pre-defines, did I miss any?Thanks for doing this. I think we can catch a few more...OK, I didn't grep accurately enough it seems. Here is a new version hopefully addressing your comments.
yes, that looks good now.
However, I'm not sure whether replacing uses of __ARM_ARCH__ and removing support for arches < 4 should be in the same patch: this goes beyond my original intent, and I've noticed probable dead code in include/longlong.h (support for umul_ppmm on arm v2 and v3)
I see your point. It could indeed be cleaner if the code removal hunk was put in a separate patch. A bugzilla entry about the dead code to be removed would be appreciated, I can take care of that then.
Similarly there is code to define __ARM_ARCH in libffi/src/arm/sysv.S.
I believe libffi is its own separate project that we import in GCC, so it may want to support compiling with older GCC versions. I'd need to double-check that.
So it seems further cleanup would be needed.
Indeed. This patch is ok with the __ARM_ARCH < 4 path removals separated into their own patch. Thanks, Kyrill
Christophediff --git a/libgcc/config/arm/ieee754-df.S b/libgcc/config/arm/ieee754-df.S index 570e5f6..7c5260e 100644 --- a/libgcc/config/arm/ieee754-df.S +++ b/libgcc/config/arm/ieee754-df.S @@ -245,7 +245,7 @@ LSYM(Lad_a): @ No rounding necessary since ip will always be 0 at this point. LSYM(Lad_l): -#if __ARM_ARCH__ < 5 +#if __ARM_ARCH < 5 This path exists to handle the case when the CLZ instruction is not available (the #else path uses CLZ). So we can change this to #ifndef __ARM_FEATURE_CLZ teq xh, #0 movne r3, #20 @@ -656,7 +656,7 @@ ARM_FUNC_ALIAS aeabi_dmul muldf3 orr yh, yh, #0x00100000 beq LSYM(Lml_1) -#if __ARM_ARCH__ < 4 +#if __ARM_ARCH < 4 We can delete this whole path as we no longer support anything older than 4 diff --git a/libgcc/config/arm/ieee754-sf.S b/libgcc/config/arm/ieee754-sf.S index dac3e2e..00a8d9c 100644 --- a/libgcc/config/arm/ieee754-sf.S +++ b/libgcc/config/arm/ieee754-sf.S @@ -175,7 +175,7 @@ LSYM(Lad_a): @ No rounding necessary since r1 will always be 0 at this point. LSYM(Lad_l): -#if __ARM_ARCH__ < 5 +#if __ARM_ARCH < 5 movs ip, r0, lsr #12 moveq r0, r0, lsl #12 @@ -370,7 +370,7 @@ ARM_FUNC_ALIAS aeabi_l2f floatdisf subeq r3, r3, #(32 << 23) 2: sub r3, r3, #(1 << 23) -#if __ARM_ARCH__ < 5 +#if __ARM_ARCH < 5 mov r2, #23 cmp ip, #(1 << 16) Similar comment on checking __ARM_FEATURE_CLZ in the above two checks. @@ -460,7 +460,7 @@ LSYM(Lml_x): orr r0, r3, r0, lsr #5 orr r1, r3, r1, lsr #5 -#if __ARM_ARCH__ < 4 +#if __ARM_ARCH < 4 @ Put sign bit in r3, which will be restored into r0 later. and r3, ip, #0x80000000 Likewise on deleting the < 4 path. diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S index 04c1b77..264d54a 100644 --- a/libgcc/config/arm/lib1funcs.S +++ b/libgcc/config/arm/lib1funcs.S @@ -74,49 +74,6 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see <snip> -#if __ARM_ARCH__ >= 5 && ! defined (__OPTIMIZE_SIZE__) +#if __ARM_ARCH >= 5 && ! defined (__OPTIMIZE_SIZE__) Likewise I believe this check should be for __ARM_FEATURE_CLZ, with the rest of the #elses updated accordingly. <snip> @@ -1701,8 +1658,8 @@ LSYM(Lover12): #if (__ARM_ARCH_ISA_THUMB == 2 \ || (__ARM_ARCH_ISA_ARM \ - && (__ARM_ARCH__ > 5 \ - || (__ARM_ARCH__ == 5 && __ARM_ARCH_ISA_THUMB)))) + && (__ARM_ARCH > 5 \ + || (__ARM_ARCH == 5 && __ARM_ARCH_ISA_THUMB)))) #define HAVE_ARM_CLZ 1 #endif This HAVE_ARM_CLZ should now be redundant as we can update its uses to __ARM_FEATURE_CLZ Thanks, Kyrill
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |