Subject: [bug#52283] [PATCH 00/10] Tuning packages for CPU micro-architectures

From: zimoun <[email-masked]>
Date: Mon, 06 Dec 2021 13:47:08 +0100

Hi Ludo,

Really cool!  Thanks!

On Mon, 06 Dec 2021 at 11:38, Ludovic Courtès <[email-masked]> wrote:
> Mathieu Othacehe <[email-masked]> skribis:

>>> +(define-record-type <cpu>
>>> +  (cpu architecture family model flags)
>>> +  cpu?
>>> +  (architecture cpu-architecture)                 ;string, from 'uname'
>>> +  (family       cpu-family)                       ;integer
>>> +  (model        cpu-model)                        ;integer
>>> +  (flags        cpu-flags))                       ;set of strings
>>
>> When using the "--tune" transformation option with "native", we can
>> expect the current-cpu method to fill the <cpu> record correctly.
>>
>> However, when the user is passing a custom cpu name, it might be
>> incorrect. I think we should check the user input against a list of
>> valid/supported cpu architectures.

[...]

> Right.  I’m a bit torn because I agree with the usability issue and
> solution you propose, but at the same time I know that maintaining a
> list of existing CPU names will be tedious and it’ll be annoying for
> users if they can’t just specify their CPU name (which they might want
> to do precisely when ‘--tune=native’ doesn’t determine the right name
> because it doesn’t know about it yet.)

I have not looked at all the details but this list of existing CPU name
is not somehow already maintained, no?

--8<---------------cut here---------------start------------->8---
 +(define (cpu->gcc-architecture cpu)
 +  "Return the architecture name, suitable for GCC's '-march' flag, that
 +corresponds to CPU, a record as returned by 'current-cpu'."
 +  (match (cpu-architecture cpu)
 +    ("x86_64"
 +     ;; Transcribed from GCC's 'host_detect_local_cpu' in driver-i386.c.
 +     (or (and (= 6 (cpu-family cpu))              ;the "Pentium Pro" family
 +              (letrec-syntax ((model (syntax-rules (=>)
 +                                       ((_) #f)
 +                                       ((_ (candidate => integers ...) rest
 +                                           ...)
 +                                        (or (and (= (cpu-model cpu) integers)
 +                                                 candidate)
 +                                            ...
 +                                            (model rest ...))))))
 +                (model ("bonnel" => #x1c #x26)
 +                       ("silvermont" => #x37 #x4a #x4d #x5a #x5d)
 +                       ("core2" => #x0f #x17 #x1d)
 +                       ("nehalem" => #x1a #x1e #x1f #x2e)
 +                       ("westmere" => #x25 #x2c #x2f)
 +                       ("sandybridge" => #x2a #x2d)
 +                       ("ivybridge" => #x3a #x3e)
 +                       ("haswell" => #x3c #x3f #x45 #x46)
 +                       ("broadwell" => #x3d #x47 #x4f #x56)
 +                       ("skylake" => #x4e #x5e #x8e #x9e)
 +                       ("skylake-avx512" => #x55) ;TODO: cascadelake
 +                       ("knl" => #x57)
 +                       ("cannonlake" => #x66)
 +                       ("knm" => #x85))))
--8<---------------cut here---------------end--------------->8---

>> Maybe the (guix cpu) and (gnu platform) modules should be merged somehow
>> to define the supported CPU micro-architectures:
>>
>> (define armv7-linux
>>   (platform
>>    (target "arm-linux-gnueabihf")
>>    (system "armhf-linux")
>>    (linux-architecture "arm")
>>    (supported-march '("armv7" "armv7-a" "armv7ve"))
>>
>> we could then use those platform records in the (gnu ci) module to build
>> packages against all the supported micro architectures and remove the
>> "%x86-64-micro-architecture" variable you propose to introduce there.
>
> Hmm yeah, but it should be (guix platforms) then…
>
> Maybe that’s a broader refactoring we can keep for later?  I agree it
> would be logical but I’m not sure how to nicely factorize things.

Yeah, I am always annoyed for the arguments of ’-s’ vs ’-t’, aside the
ugly backtrace. :-) The same (as we do elsewhere) is to somehow have
options ’--list-systems’ and ’--list-targets’ and handle incorrect
values; similar to “guix lint” for checkers or “guix graph” for types or
backends, etc.  With potentially some hints. :-)

I also agree that’s unrelated to the current series. :-) This
refactoring could happen later, IMHO.

Cheers,
simon