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

[Alice Carlotti]

On Fri, May 16, 2025 at 05:14:50PM +0100, Alice Carlotti wrote:
> On Fri, May 16, 2025 at 01:53:12PM +0100, 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.
> 
> I have just one tiny nit, but otherwise it looks good to me (though of course
> you'll need Richard's approval).

Just to add - I think it's also fine as it is if Richard approves.  There's no
point getting too picky about a few lines of code that will be deleted again in
a couple of weeks.

> 
> > ---
> >  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,
> I'd retain the AARCH64_FEATURE_ prefix here for consistency.  I'd also specify
> in the comment that the enum values are added temporarily to ensure that
> bit 128 is used.
> 
> >    /* 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, ...) \
> > -- 
> > 2.45.2
> >