On Fri, Oct 21, 2016 at 04:57:22PM +0100, Richard Earnshaw (lists) wrote: > On 21/10/16 14:59, James Greenhalgh wrote: > > 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. > > > > I don't think it's a good idea to line wrap the def files, some of them > are processed with AWK during configure and having a complete entry per > line avoids potential matching problems.
Agreed (and essential) for the entries themselves. This is just a comment that hangs over the end and should be fixed. While I'm here... > >> + where the big part number comes as the first arugment to the macro and > >> little is the s/arugment/argument. Cheers, James
