[PATCH] aarch64: Update macros for more aarch64_feature_bit enum values

[Richard Earnshaw (lists)]

On 16/05/2025 13:53, Ezra.Sitorus@arm.com wrote:
> From: Ezra Sitorus <ezra.sitorus@arm.com>
> 
> Currently, 128 bits are used to indicate whether or not the CPU
> supports a given feature. With more features being added to binutils,
> the struct aarch64_feature_set adjusts itself by adding a third
> uint64_t, which allows us to add 64 more features.
> 
> However, the macros used to work with the feature set are not set up to
> work with a bigger aarch64_feature_set struct. This patch updates the
> macros so that they use all 3 flags.
> 
> One thing to note is that this requires AARCH64_NUM_FEATURES to be at
> least 129. This has been done by prepending aarch64_feature_bit with
> fake values. As more features are added, these should be removed.
> 
> Regression tested on aarch64-none-linux-gnu.

Hi Ezra,

Thanks for the patch.  However, the number of places this patch needed to touch to add an additional feature word made me realize that all this was quite fragile and that there ought to be a better way of doing it.  So I've now committed https://sourceware.org/pipermail/binutils/2025-June/141671.html.  As you will see from patch 2 in that series, increasing the number of flags words now involves adding just one additional line of code (at least, until we decide that we want to loop over all the elements, but that's some way off yet).

We no-longer need to add dummy feature defines either.

R.

> ---
>  include/opcode/aarch64.h | 38 ++++++++++++++++++++++++++++----------
>  1 file changed, 28 insertions(+), 10 deletions(-)
> 
> diff --git a/include/opcode/aarch64.h b/include/opcode/aarch64.h
> index dfe3f05820a..c0c6cb72c7b 100644
> --- a/include/opcode/aarch64.h
> +++ b/include/opcode/aarch64.h
> @@ -41,6 +41,12 @@ typedef uint32_t aarch64_insn;
>  /* An enum containing all known CPU features.  The values act as bit positions
>     into aarch64_feature_set.  */
>  enum aarch64_feature_bit {
> +  /* Dummy features to expand aarch64_feature_set. */
> +  AARCH64_DUMMY_FEATURE_1,
> +  AARCH64_DUMMY_FEATURE_2,
> +  AARCH64_DUMMY_FEATURE_3,
> +  AARCH64_DUMMY_FEATURE_4,
> +  AARCH64_DUMMY_FEATURE_5,
>    /* All processors.  */
>    AARCH64_FEATURE_V8,
>    /* ARMv8.6 processors.  */
> @@ -436,29 +442,35 @@ typedef struct {
>  
>  #define AARCH64_CPU_HAS_FEATURE(CPU,FEAT)	\
>    ((~(CPU).flags[0] & AARCH64_FEATBIT (0, FEAT)) == 0		\
> -   && (~(CPU).flags[1] & AARCH64_FEATBIT (1, FEAT)) == 0)
> +   && (~(CPU).flags[1] & AARCH64_FEATBIT (1, FEAT)) == 0	\
> +   && (~(CPU).flags[2] & AARCH64_FEATBIT (2, FEAT)) == 0)
>  
>  #define AARCH64_CPU_HAS_ALL_FEATURES(CPU,FEAT)	\
>    ((~(CPU).flags[0] & (FEAT).flags[0]) == 0	\
> -   && (~(CPU).flags[1] & (FEAT).flags[1]) == 0)
> +   && (~(CPU).flags[1] & (FEAT).flags[1]) == 0	\
> +   && (~(CPU).flags[2] & (FEAT).flags[2]) == 0)
>  
>  #define AARCH64_CPU_HAS_ANY_FEATURES(CPU,FEAT)	\
>    (((CPU).flags[0] & (FEAT).flags[0]) != 0	\
> -   || ((CPU).flags[1] & (FEAT).flags[1]) != 0)
> +   || ((CPU).flags[1] & (FEAT).flags[1]) != 0	\
> +   || ((CPU).flags[2] & (FEAT).flags[2]) != 0)
>  
>  #define AARCH64_SET_FEATURE(DEST, FEAT) \
>    ((DEST).flags[0] = FEAT (0),		\
> -   (DEST).flags[1] = FEAT (1))
> +   (DEST).flags[1] = FEAT (1),		\
> +   (DEST).flags[2] = FEAT (2))
>  
>  #define AARCH64_CLEAR_FEATURE(DEST, SRC, FEAT)		\
>    ((DEST).flags[0] = (SRC).flags[0] & ~AARCH64_FEATBIT (0, FEAT), \
> -   (DEST).flags[1] = (SRC).flags[1] & ~AARCH64_FEATBIT (1, FEAT))
> +   (DEST).flags[1] = (SRC).flags[1] & ~AARCH64_FEATBIT (1, FEAT), \
> +   (DEST).flags[2] = (SRC).flags[2] & ~AARCH64_FEATBIT (2, FEAT))
>  
>  #define AARCH64_MERGE_FEATURE_SETS(TARG,F1,F2)		\
>    do							\
>      {							\
>        (TARG).flags[0] = (F1).flags[0] | (F2).flags[0];	\
>        (TARG).flags[1] = (F1).flags[1] | (F2).flags[1];	\
> +      (TARG).flags[2] = (F1).flags[2] | (F2).flags[2];	\
>      }							\
>    while (0)
>  
> @@ -467,24 +479,29 @@ typedef struct {
>      {							\
>        (TARG).flags[0] = (F1).flags[0] &~ (F2).flags[0];	\
>        (TARG).flags[1] = (F1).flags[1] &~ (F2).flags[1];	\
> +      (TARG).flags[2] = (F1).flags[2] &~ (F2).flags[2];	\
>      }							\
>    while (0)
>  
>  /* aarch64_feature_set initializers for no features and all features,
>     respectively.  */
> -#define AARCH64_NO_FEATURES { { 0, 0 } }
> -#define AARCH64_ALL_FEATURES { { -1, -1 } }
> +#define AARCH64_NO_FEATURES { { 0, 0, 0 } }
> +#define AARCH64_ALL_FEATURES { { -1, -1, -1 } }
>  
>  /* An aarch64_feature_set initializer for a single feature,
>     AARCH64_FEATURE_<FEAT>.  */
>  #define AARCH64_FEATURE(FEAT) \
> -  { { AARCH64_FEATBIT (0, FEAT), AARCH64_FEATBIT (1, FEAT) } }
> +  { { AARCH64_FEATBIT (0, FEAT), \
> +      AARCH64_FEATBIT (1, FEAT), \
> +      AARCH64_FEATBIT (2, FEAT) } }
>  
>  /* An aarch64_feature_set initializer for a specific architecture version,
>     including all the features that are enabled by default for that architecture
>     version.  */
>  #define AARCH64_ARCH_FEATURES(ARCH) \
> -  { { AARCH64_ARCH_##ARCH (0), AARCH64_ARCH_##ARCH (1) } }
> +  { { AARCH64_ARCH_##ARCH (0), \
> +      AARCH64_ARCH_##ARCH (1), \
> +      AARCH64_ARCH_##ARCH (2)} }
>  
>  /* Used by AARCH64_CPU_FEATURES.  */
>  #define AARCH64_OR_FEATURES_1(X, ARCH, F1) \
> @@ -510,7 +527,8 @@ typedef struct {
>     version ARCH, and additionally provides the N features listed in "...".  */
>  #define AARCH64_CPU_FEATURES(ARCH, N, ...)			\
>    { { AARCH64_OR_FEATURES_##N (0, ARCH, __VA_ARGS__),		\
> -      AARCH64_OR_FEATURES_##N (1, ARCH, __VA_ARGS__) } }
> +      AARCH64_OR_FEATURES_##N (1, ARCH, __VA_ARGS__),		\
> +      AARCH64_OR_FEATURES_##N (2, ARCH, __VA_ARGS__) } }
>  
>  /* An aarch64_feature_set initializer for the N features listed in "...".  */
>  #define AARCH64_FEATURES(N, ...) \