Re: [PATCH/AARCH64] Remove index from AARCH64_FUSION_PAIR

[James Greenhalgh <james.greenhalgh@arm.com>]

On Wed, Aug 19, 2015 at 12:11:04PM +0100, Andrew Pinski wrote:
>     Instead of doing an explicit index in aarch64-fusion-pairs.def, we
>     should have an enum which does the index instead.  This allows
>     you to add/remove them without worrying about the order being
>     correct and having holes or worry about merge conflicts.
> 
>     OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions.
> 

Looks good, it would be good to expand this patch to get rid of the
messy way we build the AARCH64_FUSE_ALL macro:

> /* Hacky macro to build AARCH64_FUSE_ALL.  The sequence below expands
>    to:
>    AARCH64_FUSE_ALL = 0 | AARCH64_FUSE_index1 | AARCH64_FUSE_index2 ...  */
> #undef AARCH64_FUSION_PAIR
> #define AARCH64_FUSION_PAIR(x, name, y) \
>   | AARCH64_FUSE_##name
> 
>   AARCH64_FUSE_ALL = 0
> #include "aarch64-fusion-pairs.def"

We should now be able to do something like:

  AARCH64_FUSE_ALL = ((1 << AARCH64_FUSE_index_END) - 1)

Right?

If so, could you respin with that change?

Thanks,
James


> diff --git a/gcc/config/aarch64/aarch64-fusion-pairs.def b/gcc/config/aarch64/aarch64-fusion-pairs.def
> index a7b00f6..53bbef4 100644
> --- a/gcc/config/aarch64/aarch64-fusion-pairs.def
> +++ b/gcc/config/aarch64/aarch64-fusion-pairs.def
> @@ -20,19 +20,17 @@
>  /* Pairs of instructions which can be fused. before including this file,
>     define a macro:
>  
> -     AARCH64_FUSION_PAIR (name, internal_name, index_bit)
> +     AARCH64_FUSION_PAIR (name, internal_name)
>  
>     Where:
>  
>       NAME is a string giving a friendly name for the instructions to fuse.
>       INTERNAL_NAME gives the internal name suitable for appending to
> -     AARCH64_FUSE_ to give an enum name.
> -     INDEX_BIT is the bit to set in the bitmask of supported fusion
> -     operations.  */
> -
> -AARCH64_FUSION_PAIR ("mov+movk", MOV_MOVK, 0)
> -AARCH64_FUSION_PAIR ("adrp+add", ADRP_ADD, 1)
> -AARCH64_FUSION_PAIR ("movk+movk", MOVK_MOVK, 2)
> -AARCH64_FUSION_PAIR ("adrp+ldr", ADRP_LDR, 3)
> -AARCH64_FUSION_PAIR ("cmp+branch", CMP_BRANCH, 4)
> +     AARCH64_FUSE_ to give an enum name. */
> +
> +AARCH64_FUSION_PAIR ("mov+movk", MOV_MOVK)
> +AARCH64_FUSION_PAIR ("adrp+add", ADRP_ADD)
> +AARCH64_FUSION_PAIR ("movk+movk", MOVK_MOVK)
> +AARCH64_FUSION_PAIR ("adrp+ldr", ADRP_LDR)
> +AARCH64_FUSION_PAIR ("cmp+branch", CMP_BRANCH)
>  
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 0b09d49..c4c1817 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -201,8 +201,18 @@ struct tune_params
>    unsigned int extra_tuning_flags;
>  };
>  
> -#define AARCH64_FUSION_PAIR(x, name, index) \
> -  AARCH64_FUSE_##name = (1 << index),
> +#define AARCH64_FUSION_PAIR(x, name) \
> +  AARCH64_FUSE_##name##_index, 
> +/* Supported fusion operations.  */
> +enum aarch64_fusion_pairs_index
> +{
> +#include "aarch64-fusion-pairs.def"
> +  AARCH64_FUSE_index_END
> +};
> +#undef AARCH64_FUSION_PAIR
> +
> +#define AARCH64_FUSION_PAIR(x, name) \
> +  AARCH64_FUSE_##name = (1 << AARCH64_FUSE_##name##_index),
>  /* Supported fusion operations.  */
>  enum aarch64_fusion_pairs
>  {
> @@ -213,7 +223,7 @@ enum aarch64_fusion_pairs
>     to:
>     AARCH64_FUSE_ALL = 0 | AARCH64_FUSE_index1 | AARCH64_FUSE_index2 ...  */
>  #undef AARCH64_FUSION_PAIR
> -#define AARCH64_FUSION_PAIR(x, name, y) \
> +#define AARCH64_FUSION_PAIR(x, name) \
>    | AARCH64_FUSE_##name
>  
>    AARCH64_FUSE_ALL = 0
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index aa268ae..162e25e 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -172,7 +172,7 @@ struct aarch64_flag_desc
>    unsigned int flag;
>  };
>  
> -#define AARCH64_FUSION_PAIR(name, internal_name, y) \
> +#define AARCH64_FUSION_PAIR(name, internal_name) \
>    { name, AARCH64_FUSE_##internal_name },
>  static const struct aarch64_flag_desc aarch64_fusible_pairs[] =
>  {