On Sat, Oct 15, 2016 at 07:38:40PM -0700, Andrew Pinski wrote:
> On Wed, Nov 25, 2015 at 11:59 AM, Andrew Pinski <pins...@gmail.com> wrote:
> Here is finally an updated (fixed) patch (I did not implement the two
> implementer big.LITTLE support yet, that will be for a different patch
> since I also fixed the part no not being unique as a separate patch.
> Once I get a new enough kernel, I will also look into doing the
> /sys/cpu/* style detection first.
>
> OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions
> (and tested hacking the location of the read in file to see if it
> works with big.LITTLE and other formats of /proc/cpuinfo).
I'm OK with this in principle, but it needs some polish for pedantic
style comments...
> * config/aarch64/aarch64-cores.def: Rewrite so IMP and PART are
> integer constants.
> * config/aarch64/driver-aarch64.c (struct aarch64_core_data): Change
> implementer_id to unsigned char.
> Change part_no to unsigned int.
> (AARCH64_BIG_LITTLE): New define.
> (INVALID_IMP): New define.
> (INVALID_CORE): New define.
> (cpu_data): Change the last element's implementer_id and part_no to integers.
> (valid_bL_string_p): Rewrite to ..
> (valid_bL_core_p): this for integers instead of strings.
> (parse_field): New function.
> (contains_string_p): Rewrite to ...
> (contains_core_p): this for integers and only for the part_no.
> (host_detect_local_cpu): Rewrite handling of implementation and part
> num to be integers;
> simplifying the code.
> Index: config/aarch64/aarch64-cores.def
> ===================================================================
> --- config/aarch64/aarch64-cores.def (revision 241200)
> +++ config/aarch64/aarch64-cores.def (working copy)
> @@ -32,43 +32,46 @@
> 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.
> - IMP is the implementer ID of the CPU vendor. On a GNU/Linux system it can
> - be found in /proc/cpuinfo.
> + IMP is the implementer ID of the CPU vendor. On a GNU/Linux system it
> + can be found in /proc/cpuinfo. A partial list of implementer IDs is
> + given in the ARM Architecture Reference Manual ARMv8, for
> - in /proc/cpuinfo. For big.LITTLE systems this should have the form at of
> - "<big core part number>.<LITTLE core part number>". */
> + in /proc/cpuinfo. For big.LITTLE systems this should use the macro
> AARCH64_BIG_LITTLE
> + where the big part number comes as the first arugment to the macro and
> little is the
> + second. */
Needs rewrapped for 80 char width.
>
> -static bool
> -valid_bL_string_p (const char** core, const char* bL_string)
> + static bool
> +valid_bL_core_p (unsigned int *core, unsigned int bL_core)
Stray space before static.
> {
> - return strstr (bL_string, core[0]) != NULL
> - && strstr (bL_string, core[1]) != NULL;
> + return AARCH64_BIG_LITTLE (core[0], core[1]) == bL_core
> + || AARCH64_BIG_LITTLE (core[1], core[0]) == bL_core;
> +}
> +
> +/* Returns the integer that is after ':' for the field. */
> +static unsigned parse_field (const char *field)
parse_field should be on a new line, FIELD should be capitalised in the
explanatory comment.
OK with the appropriate changes to rectify these points.
Thanks,
James
