[PATCHv2/AARCH64] Remove index from AARCH64_EXTRA_TUNING_OPTION

[PATCHv2/AARCH64] Remove index from AARCH64_EXTRA_TUNING_OPTION

[Andrew Pinski]

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.

    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..

Re: [PATCHv2/AARCH64] Remove index from AARCH64_EXTRA_TUNING_OPTION

[James Greenhalgh]

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.

>     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.

>     (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..

Thanks,
James

> 

Re: [PATCHv2/AARCH64] Remove index from AARCH64_EXTRA_TUNING_OPTION

[Andrew Pinski]

[-- 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[] =
 {