Re: [PATCH][AArch64][2/14] Refactor arches handling, add arch enum identifier

2015-07-17 Thread James Greenhalgh
On Thu, Jul 16, 2015 at 04:20:33PM +0100, Kyrill Tkachov wrote:
> Hi all,
> 
> In this second patch I want to get to the point where I can get an enum that
> I can use to index all_architectures to get the current architecture being
> used, similar to what we
> do in patch 1/N.
> 
> The closest thing to what I want in aarch64-arches.def is the 3rd field which
> specifies the architecture revision. Unfortunately, it is used sometimes as
> an integer and sometimes as a string when defining the __ARM_ARCH macro in
> TARGET_CPU_CPP_BUILTINS.
> 
> I've decided to create a new field that is to be used as part of an enum name
> to uniquely identify each entry in aarch64-arches.def. The revision number
> (currently only '8') is left there since we need it for the ACLE predefs, but
> we might consider moving that out in the future...
> 
> In any case, with this patch we can now get an enum that can be used to
> access the architecture information from all_architectures and can be easily
> saved and restored for SWITCHABLE_TARGET functionality.
> 
> Bootstrapped with and without LTO and tested on aarch64 as part of the series.
> 
> Ok for trunk?

Ok.

Thanks,
James

> P.S. I think we should consider creating a separate struct definition for
> cores and architectures as the information we want to store about each starts
> to diverge and it's sometimes confusing as to what a 'struct processor*'
> pointer is referencing. But such a refactoring would interfere too much with
> what I'm trying to do in this patch series and is not strictly required for
> it. Although, once the dust settles on this series, I believe it will be
> easier to split them up.

I look forward to seeing the patch ;-)

James

> 
> 2015-07-16  Kyrylo Tkachov  
> 
>  * config/aarch64/aarch64.h (TARGET_CPU_CPP_BUILTINS): Define
>  __ARM_ARCH_8A directly rather than with cpp_define_formatted.
>  * config/aarch64/aarch64.c (struct processor): Add arch field.
>  (all_architectures): Handle above, move above all_cores.
>  (all_cores): Handle above.
>  (aarch64_parse_arch): Handle above changes.
>  * config/aarch64/aarch64-arches.def (armv8-a): Extend according to
>  above.  Update comments.
>  (armv8.1-a): Likewise.
>  * config/aarch64/aarch64-cores.def: Update according to above.
>  * config/aarch64/aarch64-opts.h (aarch64_arch): New enum.
>  * config/aarch64/driver-aarch64.c (struct aarch64_arch): Rename to
>  aarch64_arch_driver_info.



[PATCH][AArch64][2/14] Refactor arches handling, add arch enum identifier

2015-07-16 Thread Kyrill Tkachov

Hi all,

In this second patch I want to get to the point where I can get an enum that I 
can use
to index all_architectures to get the current architecture being used, similar 
to what we
do in patch 1/N.

The closest thing to what I want in aarch64-arches.def is the 3rd field which 
specifies the
architecture revision. Unfortunately, it is used sometimes as an integer and 
sometimes as a string
when defining the __ARM_ARCH macro in TARGET_CPU_CPP_BUILTINS.

I've decided to create a new field that is to be used as part of an enum name 
to uniquely identify
each entry in aarch64-arches.def. The revision number (currently only '8') is 
left there since we
need it for the ACLE predefs, but we might consider moving that out in the 
future...

In any case, with this patch we can now get an enum that can be used to access 
the architecture
information from all_architectures and can be easily saved and restored for 
SWITCHABLE_TARGET functionality.

Bootstrapped with and without LTO and tested on aarch64 as part of the series.

Ok for trunk?

Thanks,
Kyrill

P.S. I think we should consider creating a separate struct definition for cores 
and architectures as
the information we want to store about each starts to diverge and it's 
sometimes confusing as to what
a 'struct processor*' pointer is referencing. But such a refactoring would 
interfere too much with what
I'm trying to do in this patch series and is not strictly required for it. 
Although, once the dust settles
on this series, I believe it will be easier to split them up.

2015-07-16  Kyrylo Tkachov  

* config/aarch64/aarch64.h (TARGET_CPU_CPP_BUILTINS): Define
__ARM_ARCH_8A directly rather than with cpp_define_formatted.
* config/aarch64/aarch64.c (struct processor): Add arch field.
(all_architectures): Handle above, move above all_cores.
(all_cores): Handle above.
(aarch64_parse_arch): Handle above changes.
* config/aarch64/aarch64-arches.def (armv8-a): Extend according to
above.  Update comments.
(armv8.1-a): Likewise.
* config/aarch64/aarch64-cores.def: Update according to above.
* config/aarch64/aarch64-opts.h (aarch64_arch): New enum.
* config/aarch64/driver-aarch64.c (struct aarch64_arch): Rename to
aarch64_arch_driver_info.
commit ea24da31afa938379e1e679de581360b05f4e0f5
Author: Kyrylo Tkachov 
Date:   Mon May 11 12:09:34 2015 +0100

[AArch64][2/N] Refactor arches handling, add arch enum identifier

diff --git a/gcc/config/aarch64/aarch64-arches.def b/gcc/config/aarch64/aarch64-arches.def
index abbfce6..3b4fb73 100644
--- a/gcc/config/aarch64/aarch64-arches.def
+++ b/gcc/config/aarch64/aarch64-arches.def
@@ -19,12 +19,17 @@
 
 /* Before using #include to read this file, define a macro:
 
-  AARCH64_ARCH(NAME, CORE, ARCH, FLAGS)
+  AARCH64_ARCH(NAME, CORE, ARCH_IDENT, ARCH_REV, FLAGS)
 
The NAME is the name of the architecture, represented as a string
constant.  The CORE is the identifier for a core representative of
-   this architecture.  ARCH is the architecture revision.  FLAGS are
-   the flags implied by the architecture.  */
+   this architecture.  ARCH_IDENT is the architecture identifier.  It must be
+   unique and be syntactically valid to appear as part of an enum identifier.
+   ARCH_REV is an integer specifying the architecture major revision.
+   FLAGS are the flags implied by the architecture.
+   Due to the assumptions about the positions of these fields in config.gcc,
+   the NAME should be kept as the first argument and FLAGS as the last.  */
+
+AARCH64_ARCH("armv8-a",	  generic,	 8A,	8,  AARCH64_FL_FOR_ARCH8)
+AARCH64_ARCH("armv8.1-a", generic,	 8_1A,	8,  AARCH64_FL_FOR_ARCH8_1)
 
-AARCH64_ARCH("armv8-a",	  generic,	 8,  AARCH64_FL_FOR_ARCH8)
-AARCH64_ARCH("armv8.1-a", generic,	 8,  AARCH64_FL_FOR_ARCH8_1)
diff --git a/gcc/config/aarch64/aarch64-cores.def b/gcc/config/aarch64/aarch64-cores.def
index c4e22fe..0ab1ca8 100644
--- a/gcc/config/aarch64/aarch64-cores.def
+++ b/gcc/config/aarch64/aarch64-cores.def
@@ -21,13 +21,14 @@
 
Before using #include to read this file, define a macro:
 
-  AARCH64_CORE(CORE_NAME, CORE_IDENT, SCHEDULER_IDENT, ARCH, FLAGS, COSTS, IMP, PART)
+  AARCH64_CORE(CORE_NAME, CORE_IDENT, SCHEDULER_IDENT, ARCH_IDENT, FLAGS, COSTS, IMP, PART)
 
The CORE_NAME is the name of the core, represented as a string constant.
The CORE_IDENT is the name of the core, represented as an identifier.
The SCHEDULER_IDENT is the name of the core for which scheduling decisions
will be made, represented as an identifier.
-   ARCH is the architecture revision implemented by the chip.
+   ARCH_IDENT is the architecture implemented by the chip as specified in
+   aarch64-arches.def.
FLAGS are the bitwise-or of the traits that apply to that core.
This need not include flags implied by the architecture.
COSTS is the name of the rtx_costs routine to use.
@@ -39,14 +40,15 @@
 
 /* V8 Architect