| Message ID | 20211204204924.15581-1-ludo@gnu.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Tuning packages for CPU micro-architectures | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/applying patch | fail | View Laminar job |
| cbaines/issue | success | View issue |
| cbaines/applying patch | fail | View Laminar job |
| cbaines/issue | success | View issue |
| cbaines/applying patch | fail | View Laminar job |
| cbaines/issue | success | View issue |
Hey Ludo, Wooh, nice addition! > +(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. That's something we should also enforce for the system and target fields. Currently, this command "guix build -s arch64-linux hello" is failing with an unpleasant backtrace, while it could warn that the given system is not supported. Maybe the (guix cpu) and (gnu platform) modules should be merged somehow to define the supported CPU micro-architectures: --8<---------------cut here---------------start------------->8--- (define armv7-linux (platform (target "arm-linux-gnueabihf") (system "armhf-linux") (linux-architecture "arm") (supported-march '("armv7" "armv7-a" "armv7ve")) --8<---------------cut here---------------end--------------->8--- 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. WDYT? Thanks, Mathieu
Hello! Mathieu Othacehe <othacehe@gnu.org> skribis: > Wooh, nice addition! Glad you like it. :-) >> +(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. > > That's something we should also enforce for the system and target > fields. Currently, this command "guix build -s arch64-linux hello" is > failing with an unpleasant backtrace, while it could warn that the > given system is not supported. 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.) Maybe it’s an acceptable limitation though. I’ll see how I can tweak the code so that the CPU detection code and the micro-architecture name validation code can share a single list of names. > 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. Thanks, Ludo’.
Hi Ludo, Really cool! Thanks! On Mon, 06 Dec 2021 at 11:38, Ludovic Courtès <ludovic.courtes@inria.fr> wrote: > Mathieu Othacehe <othacehe@gnu.org> 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
Mathieu Othacehe <othacehe@gnu.org> skribis: > 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. BTW, there’s another constraint: the list of valid names depends on the compiler used. GCC 11 recognizes ‘x86-64-v[1234]’ for instance, but earlier versions do not. We could hard-code the list of known identifiers for the default GCC, but if users resort to ‘--with-c-toolchain’ to get a newer toolchain, they won’t be able to use the newer CPU identifiers. Maybe an acceptable drawback. Ludo’.
Hey, > 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. Yes sure, I agree that this refactoring can be done later just something that we can keep in mind. Having a look to Nix, looks like they are also maintaining some kind of architecture list: https://github.com/NixOS/nixpkgs/blob/master/lib/systems/architectures.nix Thanks, Mathieu
Hi, Mathieu Othacehe <othacehe@gnu.org> skribis: > Yes sure, I agree that this refactoring can be done later just something > that we can keep in mind. Having a look to Nix, looks like they are > also maintaining some kind of architecture list: > > https://github.com/NixOS/nixpkgs/blob/master/lib/systems/architectures.nix Interesting. The list of features might be an idealized view, compared what I’ve seen in GCC. I wonder how this is supposed to be used. Their compiler wrapper (build-support/cc-wrapper/default.nix) passes ‘-march=${targetPlatform.gcc.arch}’ so maybe users can somehow override that ‘gcc.arch’ attribute? Any Nix-savvy person here? It also has a nice compatibility list: --8<---------------cut here---------------start------------->8--- # older compilers (for example bootstrap's GCC 5) fail with -march=too-modern-cpu isGccArchSupported = arch: if isGNU then { # Intel skylake = versionAtLeast ccVersion "6.0"; skylake-avx512 = versionAtLeast ccVersion "6.0"; cannonlake = versionAtLeast ccVersion "8.0"; icelake-client = versionAtLeast ccVersion "8.0"; icelake-server = versionAtLeast ccVersion "8.0"; cascadelake = versionAtLeast ccVersion "9.0"; cooperlake = versionAtLeast ccVersion "10.0"; tigerlake = versionAtLeast ccVersion "10.0"; knm = versionAtLeast ccVersion "8.0"; # AMD znver1 = versionAtLeast ccVersion "6.0"; znver2 = versionAtLeast ccVersion "9.0"; znver3 = versionAtLeast ccVersion "11.0"; }.${arch} or true else if isClang then { # Intel cannonlake = versionAtLeast ccVersion "5.0"; icelake-client = versionAtLeast ccVersion "7.0"; icelake-server = versionAtLeast ccVersion "7.0"; knm = versionAtLeast ccVersion "7.0"; # AMD znver1 = versionAtLeast ccVersion "4.0"; znver2 = versionAtLeast ccVersion "9.0"; }.${arch} or true else false; --8<---------------cut here---------------end--------------->8--- The compiler wrapper in this patch series doesn’t know what compiler it’s wrapping (it’s just calling the next one in $PATH), so it can’t really do this sort of things. We could do it differently but I liked the simplicity of just dropping the wrapper in front of $PATH. Thanks, Ludo’.
diff --git a/Makefile.am b/Makefile.am index f7e7b5184f..0846818cc2 100644 --- a/Makefile.am +++ b/Makefile.am @@ -81,6 +81,7 @@ MODULES = \ guix/base64.scm \ guix/ci.scm \ guix/cpio.scm \ + guix/cpu.scm \ guix/deprecation.scm \ guix/docker.scm \ guix/records.scm \ diff --git a/guix/cpu.scm b/guix/cpu.scm new file mode 100644 index 0000000000..77efac92a2 --- /dev/null +++ b/guix/cpu.scm @@ -0,0 +1,137 @@ +;;; GNU Guix --- Functional package management for GNU +;;; Copyright © 2021 Ludovic Courtès <ludo@gnu.org> +;;; +;;; This file is part of GNU Guix. +;;; +;;; GNU Guix is free software; you can redistribute it and/or modify it +;;; under the terms of the GNU General Public License as published by +;;; the Free Software Foundation; either version 3 of the License, or (at +;;; your option) any later version. +;;; +;;; GNU Guix is distributed in the hope that it will be useful, but +;;; WITHOUT ANY WARRANTY; without even the implied warranty of +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;;; GNU General Public License for more details. +;;; +;;; You should have received a copy of the GNU General Public License +;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>. + +(define-module (guix cpu) + #:use-module (guix sets) + #:use-module (guix memoization) + #:use-module (srfi srfi-1) + #:use-module (srfi srfi-9) + #:use-module (ice-9 match) + #:use-module (ice-9 rdelim) + #:export (current-cpu + cpu? + cpu-architecture + cpu-family + cpu-model + cpu-flags + + cpu->gcc-architecture)) + +;;; Commentary: +;;; +;;; This module provides tools to determine the micro-architecture supported +;;; by the CPU and to map it to a name known to GCC's '-march'. +;;; +;;; Code: + +;; CPU description. +(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 + +(define current-cpu + (mlambda () + "Return a <cpu> record representing the host CPU." + (define (prefix? prefix) + (lambda (str) + (string-prefix? prefix str))) + + (call-with-input-file "/proc/cpuinfo" + (lambda (port) + (let loop ((family #f) + (model #f)) + (match (read-line port) + ((? eof-object?) + #f) + ((? (prefix? "cpu family") str) + (match (string-tokenize str) + (("cpu" "family" ":" family) + (loop (string->number family) model)))) + ((? (prefix? "model") str) + (match (string-tokenize str) + (("model" ":" model) + (loop family (string->number model))) + (_ + (loop family model)))) + ((? (prefix? "flags") str) + (match (string-tokenize str) + (("flags" ":" flags ...) + (cpu (utsname:machine (uname)) + family model (list->set flags))))) + (_ + (loop family model)))))))) + +(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)))) + + ;; Fallback case for non-Intel processors or for Intel processors not + ;; recognized above. + (letrec-syntax ((if-flags (syntax-rules (=>) + ((_) + #f) + ((_ (flags ... => name) rest ...) + (if (every (lambda (flag) + (set-contains? (cpu-flags cpu) + flag)) + '(flags ...)) + name + (if-flags rest ...)))))) + (if-flags ("avx512" => "knl") + ("adx" => "broadwell") + ("avx2" => "haswell") + ;; TODO: tigerlake, cooperlake, etc. + ("avx" => "sandybridge") + ("sse4_2" "movbe" => "silvermont") + ("sse4_2" => "nehalem") + ("ssse3" "movbe" => "bonnell") + ("ssse3" => "core2"))) + "x86_64")) + (architecture + ;; TODO: AArch64. + architecture)))