| Message ID | CA+=Sn1=xt4omNS-9aoVS7_VbKTZHjYnrCw=ROgcONyF8ScMpbA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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[] = > {
On Wed, Aug 19, 2015 at 7:39 PM, James Greenhalgh <james.greenhalgh@arm.com> wrote: > 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? Yes I actually thought of that after I had submitted the patch. > > If so, could you respin with that change? Respinning this patch and the one for AARCH64_EXTRA_TUNING_OPTION. Thanks, Andrew > > 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[] = >> { >
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[] = {
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. ChangeLog: * aarch64-fusion-pairs.def: Remove all index to AARCH64_FUSION_PAIR. * config/aarch64/aarch64-protos.h (aarch64_fusion_pairs_index): New enum. (aarch64_fusion_pairs): Base the shifted value on the index instead of the argument to AARCH64_FUSION_PAIR. * config/aarch64/aarch64.c: Remove the last argument to AARCH64_FUSION_PAIR. commit 69a828bfdcd2f4de2c9d4f27e3878213d04ed353 Author: Andrew Pinski <apinski@cavium.com> Date: Tue Aug 18 22:13:32 2015 -0700 Remove index from AARCH64_FUSION_PAIR Instead of doing an explict 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 conficts. OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions. ChangeLog: * aarch64-fusion-pairs.def: Remove all index to AARCH64_FUSION_PAIR. * config/aarch64/aarch64-protos.h (aarch64_fusion_pairs_index): New enum. (aarch64_fusion_pairs): Base the shifted value on the index instead of the argument to AARCH64_FUSION_PAIR. * config/aarch64/aarch64.c: Remove the last argument to AARCH64_FUSION_PAIR.