* [PATCH/AARCH64] Remove index from AARCH64_FUSION_PAIR
@ 2015-08-19 11:15 Andrew Pinski
2015-08-19 12:11 ` James Greenhalgh
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Pinski @ 2015-08-19 11:15 UTC (permalink / raw)
To: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 704 bytes --]
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.
[-- Attachment #2: fusion.diff.txt --]
[-- Type: text/plain, Size: 3694 bytes --]
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.
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[] =
{
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH/AARCH64] Remove index from AARCH64_FUSION_PAIR
2015-08-19 11:15 [PATCH/AARCH64] Remove index from AARCH64_FUSION_PAIR Andrew Pinski
@ 2015-08-19 12:11 ` James Greenhalgh
2015-08-19 13:00 ` Andrew Pinski
0 siblings, 1 reply; 3+ messages in thread
From: James Greenhalgh @ 2015-08-19 12:11 UTC (permalink / raw)
To: Andrew Pinski; +Cc: GCC Patches
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[] =
> {
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH/AARCH64] Remove index from AARCH64_FUSION_PAIR
2015-08-19 12:11 ` James Greenhalgh
@ 2015-08-19 13:00 ` Andrew Pinski
0 siblings, 0 replies; 3+ messages in thread
From: Andrew Pinski @ 2015-08-19 13:00 UTC (permalink / raw)
To: James Greenhalgh; +Cc: GCC Patches
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[] =
>> {
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-08-19 12:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-19 11:15 [PATCH/AARCH64] Remove index from AARCH64_FUSION_PAIR Andrew Pinski
2015-08-19 12:11 ` James Greenhalgh
2015-08-19 13:00 ` Andrew Pinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).