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

["Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>]

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.

>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.
>   */
>