Re: [PATCH][ARM] Use __ARM_ARCH instead of __ARM_ARCH__

[Christophe Lyon <christophe dot lyon at linaro dot org>]

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.txt
> >
>
> Thanks, 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,

Christophe

> From a quick look it looks like the only one we can't really support
> with the new tests is the check for DMB.  I'll have a chat with the ACLE
> maintainers about that...
>
> R.
>
> >
> > diff --git a/libatomic/config/arm/arm-config.h b/libatomic/config/arm/arm-config.h
> > index c0504be..ce8ff0e 100644
> > --- a/libatomic/config/arm/arm-config.h
> > +++ b/libatomic/config/arm/arm-config.h
> > @@ -23,57 +23,15 @@
> >     <http://www.gnu.org/licenses/>.  */
> >
> >
> > -#if defined(__ARM_ARCH_2__)
> > -# define __ARM_ARCH__ 2
> > -#endif
> > -
> > -#if defined(__ARM_ARCH_3__)
> > -# define __ARM_ARCH__ 3
> > -#endif
> > -
> > -#if defined(__ARM_ARCH_3M__) || defined(__ARM_ARCH_4__) \
> > -     || defined(__ARM_ARCH_4T__)
> > -/* We use __ARM_ARCH__ set to 4 here, but in reality it's any processor with
> > -   long multiply instructions.  That includes v3M.  */
> > -# define __ARM_ARCH__ 4
> > -#endif
> > -
> > -#if defined(__ARM_ARCH_5__) || defined(__ARM_ARCH_5T__) \
> > -     || defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \
> > -     || defined(__ARM_ARCH_5TEJ__)
> > -# define __ARM_ARCH__ 5
> > -#endif
> > -
> > -#if defined(__ARM_ARCH_6__) || defined(__ARM_ARCH_6J__) \
> > -     || defined(__ARM_ARCH_6K__) || defined(__ARM_ARCH_6Z__) \
> > -     || defined(__ARM_ARCH_6ZK__) || defined(__ARM_ARCH_6T2__) \
> > -     || defined(__ARM_ARCH_6M__)
> > -# define __ARM_ARCH__ 6
> > -#endif
> > -
> > -#if defined(__ARM_ARCH_7__) || defined(__ARM_ARCH_7A__) \
> > -     || defined(__ARM_ARCH_7R__) || defined(__ARM_ARCH_7M__) \
> > -     || defined(__ARM_ARCH_7EM__)
> > -# define __ARM_ARCH__ 7
> > -#endif
> > -
> > -#if defined(__ARM_ARCH_8A__)
> > -# define __ARM_ARCH__ 8
> > -#endif
> > -
> > -#ifndef __ARM_ARCH__
> > -#error Unable to determine architecture.
> > -#endif
> > -
> > -#if __ARM_ARCH__ >= 7 || defined(__ARM_ARCH_6K__) || defined(__ARM_ARCH_6ZK__)
> > +#if __ARM_ARCH >= 7 || defined(__ARM_ARCH_6K__) || defined(__ARM_ARCH_6ZK__)
> >  # define HAVE_STREX  1
> >  # define HAVE_STREXBHD       1
> > -#elif __ARM_ARCH__ == 6
> > +#elif __ARM_ARCH == 6
> >  # define HAVE_STREX  1
> >  #endif
> >
> > -#if __ARM_ARCH__ >= 7
> > +#if __ARM_ARCH >= 7
> >  # define HAVE_DMB    1
> > -#elif __ARM_ARCH__ == 6
> > +#elif __ARM_ARCH == 6
> >  # define HAVE_DMB_MCR        1
> >  #endif
> > 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
> >
> >  /* Function end macros.  Variants for interworking.  */
> >
> > -#if defined(__ARM_ARCH_2__)
> > -# define __ARM_ARCH__ 2
> > -#endif
> > -
> > -#if defined(__ARM_ARCH_3__)
> > -# define __ARM_ARCH__ 3
> > -#endif
> > -
> > -#if defined(__ARM_ARCH_3M__) || defined(__ARM_ARCH_4__) \
> > -     || defined(__ARM_ARCH_4T__)
> > -/* We use __ARM_ARCH__ set to 4 here, but in reality it's any processor with
> > -   long multiply instructions.  That includes v3M.  */
> > -# define __ARM_ARCH__ 4
> > -#endif
> > -
> > -#if defined(__ARM_ARCH_5__) || defined(__ARM_ARCH_5T__) \
> > -     || defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \
> > -     || defined(__ARM_ARCH_5TEJ__)
> > -# define __ARM_ARCH__ 5
> > -#endif
> > -
> > -#if defined(__ARM_ARCH_6__) || defined(__ARM_ARCH_6J__) \
> > -     || defined(__ARM_ARCH_6K__) || defined(__ARM_ARCH_6Z__) \
> > -     || defined(__ARM_ARCH_6ZK__) || defined(__ARM_ARCH_6T2__) \
> > -     || defined(__ARM_ARCH_6M__)
> > -# define __ARM_ARCH__ 6
> > -#endif
> > -
> > -#if defined(__ARM_ARCH_7__) || defined(__ARM_ARCH_7A__) \
> > -     || defined(__ARM_ARCH_7R__) || defined(__ARM_ARCH_7M__) \
> > -     || defined(__ARM_ARCH_7EM__)
> > -# define __ARM_ARCH__ 7
> > -#endif
> > -
> > -#if defined(__ARM_ARCH_8A__) || defined(__ARM_ARCH_8M_BASE__) \
> > -     || defined(__ARM_ARCH_8M_MAIN__) || defined(__ARM_ARCH_8R__)
> > -# define __ARM_ARCH__ 8
> > -#endif
> > -
> > -#ifndef __ARM_ARCH__
> > -#error Unable to determine architecture.
> > -#endif
> > -
> >  /* There are times when we might prefer Thumb1 code even if ARM code is
> >     permitted, for example, the code might be smaller, or there might be
> >     interworking problems with switching to ARM state if interworking is
> > @@ -135,13 +92,13 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> >
> >  /* How to return from a function call depends on the architecture variant.  */
> >
> > -#if (__ARM_ARCH__ > 4) || defined(__ARM_ARCH_4T__)
> > +#if (__ARM_ARCH > 4) || defined(__ARM_ARCH_4T__)
> >
> >  # define RET         bx      lr
> >  # define RETc(x)     bx##x   lr
> >
> >  /* Special precautions for interworking on armv4t.  */
> > -# if (__ARM_ARCH__ == 4)
> > +# if (__ARM_ARCH == 4)
> >
> >  /* Always use bx, not ldr pc.  */
> >  #  if (defined(__thumb__) || defined(__THUMB_INTERWORK__))
> > @@ -544,7 +501,7 @@ pc                .req    r15
> >  /* ------------------------------------------------------------------------ */
> >  .macro ARM_DIV_BODY dividend, divisor, result, curbit
> >
> > -#if __ARM_ARCH__ >= 5 && ! defined (__OPTIMIZE_SIZE__)
> > +#if __ARM_ARCH >= 5 && ! defined (__OPTIMIZE_SIZE__)
> >
> >  #if defined (__thumb2__)
> >       clz     \curbit, \dividend
> > @@ -584,8 +541,8 @@ pc                .req    r15
> >       .endr
> >  #endif
> >
> > -#else /* __ARM_ARCH__ < 5 || defined (__OPTIMIZE_SIZE__) */
> > -#if __ARM_ARCH__ >= 5
> > +#else /* __ARM_ARCH < 5 || defined (__OPTIMIZE_SIZE__) */
> > +#if __ARM_ARCH >= 5
> >
> >       clz     \curbit, \divisor
> >       clz     \result, \dividend
> > @@ -595,7 +552,7 @@ pc                .req    r15
> >       mov     \curbit, \curbit, lsl \result
> >       mov     \result, #0
> >
> > -#else /* __ARM_ARCH__ < 5 */
> > +#else /* __ARM_ARCH < 5 */
> >
> >       @ Initially shift the divisor left 3 bits if possible,
> >       @ set curbit accordingly.  This allows for curbit to be located
> > @@ -626,7 +583,7 @@ pc                .req    r15
> >
> >       mov     \result, #0
> >
> > -#endif /* __ARM_ARCH__ < 5 */
> > +#endif /* __ARM_ARCH < 5 */
> >
> >       @ Division loop
> >  1:   cmp     \dividend, \divisor
> > @@ -651,13 +608,13 @@ pc              .req    r15
> >       movne   \divisor,  \divisor, lsr #4
> >       bne     1b
> >
> > -#endif /* __ARM_ARCH__ < 5 || defined (__OPTIMIZE_SIZE__) */
> > +#endif /* __ARM_ARCH < 5 || defined (__OPTIMIZE_SIZE__) */
> >
> >  .endm
> >  /* ------------------------------------------------------------------------ */
> >  .macro ARM_DIV2_ORDER divisor, order
> >
> > -#if __ARM_ARCH__ >= 5
> > +#if __ARM_ARCH >= 5
> >
> >       clz     \order, \divisor
> >       rsb     \order, \order, #31
> > @@ -687,7 +644,7 @@ pc                .req    r15
> >  /* ------------------------------------------------------------------------ */
> >  .macro ARM_MOD_BODY dividend, divisor, order, spare
> >
> > -#if __ARM_ARCH__ >= 5 && ! defined (__OPTIMIZE_SIZE__)
> > +#if __ARM_ARCH >= 5 && ! defined (__OPTIMIZE_SIZE__)
> >
> >       clz     \order, \divisor
> >       clz     \spare, \dividend
> > @@ -702,15 +659,15 @@ pc              .req    r15
> >       subcs   \dividend, \dividend, \divisor, lsl #shift
> >       .endr
> >
> > -#else /* __ARM_ARCH__ < 5 || defined (__OPTIMIZE_SIZE__) */
> > -#if __ARM_ARCH__ >= 5
> > +#else /* __ARM_ARCH < 5 || defined (__OPTIMIZE_SIZE__) */
> > +#if __ARM_ARCH >= 5
> >
> >       clz     \order, \divisor
> >       clz     \spare, \dividend
> >       sub     \order, \order, \spare
> >       mov     \divisor, \divisor, lsl \order
> >
> > -#else /* __ARM_ARCH__ < 5 */
> > +#else /* __ARM_ARCH < 5 */
> >
> >       mov     \order, #0
> >
> > @@ -732,7 +689,7 @@ pc                .req    r15
> >       addlo   \order, \order, #1
> >       blo     1b
> >
> > -#endif /* __ARM_ARCH__ < 5 */
> > +#endif /* __ARM_ARCH < 5 */
> >
> >       @ Perform all needed substractions to keep only the reminder.
> >       @ Do comparisons in batch of 4 first.
> > @@ -770,7 +727,7 @@ pc                .req    r15
> >       subhs   \dividend, \dividend, \divisor
> >  5:
> >
> > -#endif /* __ARM_ARCH__ < 5 || defined (__OPTIMIZE_SIZE__) */
> > +#endif /* __ARM_ARCH < 5 || defined (__OPTIMIZE_SIZE__) */
> >
> >  .endm
> >  /* ------------------------------------------------------------------------ */
> > @@ -1560,7 +1517,7 @@ LSYM(Lover12):
> >  @ EABI GNU/Linux call to cacheflush syscall.
> >       ARM_FUNC_START clear_cache
> >       do_push {r7}
> > -#if __ARM_ARCH__ >= 7 || defined(__ARM_ARCH_6T2__)
> > +#if __ARM_ARCH >= 7 || defined(__ARM_ARCH_6T2__)
> >       movw    r7, #2
> >       movt    r7, #0xf
> >  #else
> > @@ -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
> >
> > @@ -1887,7 +1844,7 @@ ARM_FUNC_START ctzsi2
> >     not support Thumb instructions.  (This can be a multilib option).  */
> >  #if defined __ARM_ARCH_4T__ || defined __ARM_ARCH_5T__\
> >        || defined __ARM_ARCH_5TE__ || defined __ARM_ARCH_5TEJ__ \
> > -      || __ARM_ARCH__ >= 6
> > +      || __ARM_ARCH >= 6
> >
> >  #if defined L_call_via_rX
> >
> > diff --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
> >
> >       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
> >
> >       @ Put sign bit in r6, which will be restored in yl later.
> >       and   r6, r6, #0x80000000
> > 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)
> > @@ -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
> > diff --git a/libgcc/config/arm/libunwind.S b/libgcc/config/arm/libunwind.S
> > index 3302447..50c58dc 100644
> > --- a/libgcc/config/arm/libunwind.S
> > +++ b/libgcc/config/arm/libunwind.S
> > @@ -46,7 +46,7 @@
> >       EQUIV SYM (\name), SYM (__\name)
> >  .endm
> >
> > -#if (__ARM_ARCH__ == 4)
> > +#if (__ARM_ARCH == 4)
> >  /* Some coprocessors require armv5t.  We know this code will never be run on
> >     other cpus.  Tell gas to allow armv5t, but only mark the objects as armv4.
> >   */
> >
>

Attachment: ARM_ARCH-v2.chlog.txt
Description: Text document

Attachment: ARM_ARCH-v2.patch.txt
Description: Text document