* Re: [PATCHv2/AARCH64] Remove index from AARCH64_EXTRA_TUNING_OPTION
2015-08-19 16:14 ` James Greenhalgh
@ 2015-08-22 10:36 ` Andrew Pinski
0 siblings, 0 replies; 3+ messages in thread
From: Andrew Pinski @ 2015-08-22 10:36 UTC (permalink / raw)
To: James Greenhalgh; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 1992 bytes --]
On Thu, Aug 20, 2015 at 12:08 AM, James Greenhalgh
<james.greenhalgh@arm.com> wrote:
> On Wed, Aug 19, 2015 at 05:00:14PM +0100, Andrew Pinski wrote:
>> Just like the patch for AARCH64_FUSION_PAIR, this is a patch for
>> AARCH64_EXTRA_TUNING_OPTION. Note I tested this patch on top of the
>> patch for AARCH64_EXTRA_TUNING_OPTION.
>>
>> Changes in v2:
>> Remove the hack for AARCH64_EXTRA_TUNE_ALL.
>>
>> Remove index from AARCH64_EXTRA_TUNING_OPTION
>>
>> Instead of doing an explict index in aarch64-tuning-flags.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.
>
> -ENOPATCH, but assuming this is along the same lines as the one I just
> acked, I'm happy for you to consider this preapproved (after checking
> the comments below). Please send a copy to the list for the archives.
Attached. Yes I just had forgot to attach it when I sent this one out.
>
>> ChangeLog:
>> * config/aarch64/aarch64-tuning-flags.def: Remove all index to
>> AARCH64_EXTRA_TUNING_OPTION.
>> * config/aarch64/aarch64-protos.h (extra_tuning_flags_index): New enum.
>
> I'm guessing that this has a more aarch64-centric name like
> aarch64_extra_tuning_flags_index ? If not, it probably should have just to
> fit with the naming scheme in the rest of the file.
That was just a copy and paste error into the ChangeLog.
The name in the aarch64-protos.h is aarch64_extra_tuning_flags_index.
>
>> (aarch64_extra_tuning_flags): Base the shifted value on the index instead
>> of the argument to AARCH64_EXTRA_TUNING_OPTION.
>> * config/aarch64/aarch64.c: Remove the last argument to
>> AARCH64_EXTRA_TUNING_OPTION..
>
> Watch out for the extra . on the end of this ChangeLog line..
Got it will fix.
Thanks,
Andrew
>
> Thanks,
> James
>
>>
[-- Attachment #2: tuning.diff.txt --]
[-- Type: text/plain, Size: 3755 bytes --]
commit b109e73f5966827d99b9abd8c9c45dc897604ccd
Author: Andrew Pinski <apinski@cavium.com>
Date: Wed Aug 19 01:15:00 2015 -0700
Remove index from AARCH64_EXTRA_TUNING_OPTION
Instead of doing an explict index in aarch64-tuning-flags.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:
* config/aarch64/aarch64-tuning-flags.def: Remove all index to AARCH64_EXTRA_TUNING_OPTION.
* config/aarch64/aarch64-protos.h (extra_tuning_flags_index): New enum.
(aarch64_extra_tuning_flags): Base the shifted value on the index instead
of the argument to AARCH64_EXTRA_TUNING_OPTION.
* config/aarch64/aarch64.c: Remove the last argument to AARCH64_EXTRA_TUNING_OPTION..
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 057d4fc..a12dec5 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -222,21 +222,25 @@ enum aarch64_fusion_pairs
};
#undef AARCH64_FUSION_PAIR
-#define AARCH64_EXTRA_TUNING_OPTION(x, name, index) \
- AARCH64_EXTRA_TUNE_##name = (1 << index),
+#define AARCH64_EXTRA_TUNING_OPTION(x, name) \
+ AARCH64_EXTRA_TUNE_##name##_index,
+/* Supported tuning flags indexes. */
+enum aarch64_extra_tuning_flags_index
+{
+#include "aarch64-tuning-flags.def"
+ AARCH64_EXTRA_TUNE_index_END
+};
+#undef AARCH64_EXTRA_TUNING_OPTION
+
+
+#define AARCH64_EXTRA_TUNING_OPTION(x, name) \
+ AARCH64_EXTRA_TUNE_##name = (1u << AARCH64_EXTRA_TUNE_##name##_index),
/* Supported tuning flags. */
enum aarch64_extra_tuning_flags
{
AARCH64_EXTRA_TUNE_NONE = 0,
#include "aarch64-tuning-flags.def"
-
-/* Hacky macro to build the "all" flag mask.
- Expands to 0 | AARCH64_TUNE_index0 | AARCH64_TUNE_index1 , etc. */
-#undef AARCH64_EXTRA_TUNING_OPTION
-#define AARCH64_EXTRA_TUNING_OPTION(x, name, y) \
- | AARCH64_EXTRA_TUNE_##name
- AARCH64_EXTRA_TUNE_ALL = 0
-#include "aarch64-tuning-flags.def"
+ AARCH64_EXTRA_TUNE_ALL = (1u << AARCH64_EXTRA_TUNE_index_END) - 1
};
#undef AARCH64_EXTRA_TUNING_OPTION
diff --git a/gcc/config/aarch64/aarch64-tuning-flags.def b/gcc/config/aarch64/aarch64-tuning-flags.def
index 01aaca8..628386b 100644
--- a/gcc/config/aarch64/aarch64-tuning-flags.def
+++ b/gcc/config/aarch64/aarch64-tuning-flags.def
@@ -20,15 +20,13 @@
/* Additional control over certain tuning parameters. Before including
this file, define a macro:
- AARCH64_EXTRA_TUNING_OPTION (name, internal_name, index_bit)
+ AARCH64_EXTRA_TUNING_OPTION (name, internal_name)
Where:
NAME is a string giving a friendly name for the tuning flag.
INTERNAL_NAME gives the internal name suitable for appending to
- AARCH64_TUNE_ to give an enum name.
- INDEX_BIT is the bit to set in the bitmask of supported tuning
- flags. */
+ AARCH64_TUNE_ to give an enum name. */
-AARCH64_EXTRA_TUNING_OPTION ("rename_fma_regs", RENAME_FMA_REGS, 0)
+AARCH64_EXTRA_TUNING_OPTION ("rename_fma_regs", RENAME_FMA_REGS)
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 162e25e..ad144fe 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -183,7 +183,7 @@ static const struct aarch64_flag_desc aarch64_fusible_pairs[] =
};
#undef AARCH64_FUION_PAIR
-#define AARCH64_EXTRA_TUNING_OPTION(name, internal_name, y) \
+#define AARCH64_EXTRA_TUNING_OPTION(name, internal_name) \
{ name, AARCH64_EXTRA_TUNE_##internal_name },
static const struct aarch64_flag_desc aarch64_tuning_flags[] =
{
^ permalink raw reply [flat|nested] 3+ messages in thread