[bug#64188] [PATCH 0/8] More package tuning

[bug#64188] [PATCH 0/8] More package tuning

[Efraim Flashner]

with gcc-11, gcc gained support for using -march=x86_64-v{1,2,3,4},
which I'm calling 'generic options,' as opposed to the more targeted
tuning we have with specific architectures.

Unfortunately there doesn't seem to be a way to map our current
architectures to these ones, making it harder to swap between them. The
best I've found is that any architecture which can use the Haswell
architecture optimizations can also use x86_64-v3. I suppose we could
create a mapping so that, as a fallback, anything haswell or higher
would use x86_64-v3, anything else would use x86_64-v1. This would help
with --tune=native.

For the second patch, 'Add inexact cpu matching,' I'm unsure about
needing to check for the avx512f flag; I don't have any hardware to
test with.

Patches 3 and 4 I tested on my machine (with commenting out the AMD
branch) and it successfully decided I should use x86_64-v3.

go cpu tuning targets: I mostly used the chart¹ on the go website, and I
also checked the source code for go-1.18. I put in arm{5,6,7} as arm and
not armhf since armhf only works with armv7 and with go programs, since
they're statically linked, they can just be copied to other machines.

The 6th patch, adjusting the transformations to also do go packages, I
tested with syncthing.  with '--tune' it failed during the added phase,
trying to tune for znver2, with '--tune=x86_64-v3' it worked without
problems.

Having written this out, I think our best bet would be to use a
generalized >=haswell -> x86_64-v3, else x86_64-v1, and not worry about
individual micro-architectures and specific chipsets. That would ensure
'--tune=native' should just work with go packages.

As far as tuning go packages, my understanding is that pretty much every
non-trivial go package can benefit from tuning.

EDIT:

I added two more patches on-top of the initial 6 to implement
gcc-architecture->generic-architecture and then use it. For patch 7, the
other option I had instead of returning gcc-architecture on no-match
would be to return "generic".

¹ https://github.com/golang/go/wiki/MinimumRequirements#microarchitecture-support

Efraim Flashner (8):
  gnu: %gcc-11-x86_64-micro-architectures: Add generic options.
  guix: cpu: Add inexact CPU matching.
  guix: cpu: Rewrite fallback for x86_64 cpu->gcc-architecture.
  guix: cpu: Refactor cpu->gcc-architecture.
  gnu: go: Add CPU tuning targets.
  transformations: Allow tuning go packages.
  guix: cpu: Add gcc-architecture->generic-architecture mapping.
  transformations: Allow autotuning for go packages.

 gnu/packages/gcc.scm     |   4 +-
 gnu/packages/golang.scm  |  23 ++++++-
 guix/cpu.scm             | 128 ++++++++++++++++++++++-----------------
 guix/transformations.scm |  43 +++++++++++--
 4 files changed, 135 insertions(+), 63 deletions(-)


base-commit: d884fc9e2efecfba09af4694f5a13ad7fc6f704f
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH 1/8] gnu: %gcc-11-x86_64-micro-architectures: Add generic options.

[Efraim Flashner]

* gnu/packages/gcc.scm (%gcc-11-x86_64-micro-architectures): Add entries
for x86_64-v{1,2,3,4}.
---
 gnu/packages/gcc.scm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/gcc.scm b/gnu/packages/gcc.scm
index 1b444c2b02..f5736de993 100644
--- a/gnu/packages/gcc.scm
+++ b/gnu/packages/gcc.scm
@@ -614,7 +614,9 @@ (define %gcc-11-x86_64-micro-architectures
   (append %gcc-10-x86_64-micro-architectures
           '("sapphirerapids" "alterlake" "rocketlake" ;Intel
 
-            "btver1" "btver2")))                  ;AMD
+            "btver1" "btver2"                     ;AMD
+
+            "x86_64-v1" "x86_64-v2" "x86_64-v3" "x86_64-v4")))  ; Generic
 
 ;; Suitable '-march' values for GCC 12.
 (define %gcc-12-aarch64-micro-architectures
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH 2/8] guix: cpu: Add inexact CPU matching.

[Efraim Flashner]

* guix/cpu.scm (cpu->generic-architecture): New variable.
---
 guix/cpu.scm | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/guix/cpu.scm b/guix/cpu.scm
index 45e1abeed7..e6102e3d14 100644
--- a/guix/cpu.scm
+++ b/guix/cpu.scm
@@ -32,7 +32,8 @@ (define-module (guix cpu)
             cpu-model
             cpu-flags
 
-            cpu->gcc-architecture))
+            cpu->gcc-architecture
+            cpu->generic-architecture))
 
 ;;; Commentary:
 ;;;
@@ -285,3 +286,36 @@ (define (cpu->gcc-architecture cpu)
     (architecture
      ;; TODO: More architectures
      architecture)))
+
+(define (cpu->generic-architecture cpu)
+  "Return the architecture name, suitable for inexact architecture optimizations,
+that corresponds to CPU, a record as returned by 'current-cpu'."
+  (match (cpu-architecture cpu)
+    ("x86_64"
+     (or (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
+             ;; https://gitlab.com/x86-psABIs/x86-64-ABI/-/blob/master/x86-64-ABI/low-level-sys-info.tex
+             ;; v4: AVX512F, AVX512BW, AVX512CD, AVX512DQ, AVX512VL
+             ;; v3: AVX, AVX2, BMI1, BMI2, F16C, FMA, LZCNT, MOVBE, OSXSAVE
+             ;; v2: CMPXCHG16B, LAHF, SAHF, POPCNT, SSE3, SSE4.1, SSE4.2, SSSE3
+             ("avx512f" "avx512bw" "abx512cd" "abx512dq" "avx512vl"
+              "avx" "avx2" "bmi1" "bmi2" "f16c" "fma" "movbe"
+              "popcnt" "sse3" "sse4_1" "sse4_2" "ssse3" => "x86_64-v4")
+             ("avx" "avx2" "bmi1" "bmi2" "f16c" "fma" "movbe"
+              "popcnt" "sse3" "sse4_1" "sse4_2" "ssse3" => "x86_64-v3")
+             ("popcnt" "sse3" "sse4_1" "sse4_2" "ssse3" => "x86_64-v2")
+             (_ => "x86_64-v1")))
+         "x86_64-v1"))
+    (architecture
+     ;; TODO: More architectures
+     architecture)))
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH 3/8] guix: cpu: Rewrite fallback for x86_64 cpu->gcc-architecture.

[Efraim Flashner]

* guix/cpu.scm (cpu->gcc-architecture): Adjust the fallback case to use
cpu->generic-architecture.
---
 guix/cpu.scm | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/guix/cpu.scm b/guix/cpu.scm
index e6102e3d14..e500664503 100644
--- a/guix/cpu.scm
+++ b/guix/cpu.scm
@@ -210,21 +210,7 @@ (define (cpu->gcc-architecture cpu)
            (if (and (= 7 (cpu-family cpu))
                     (= #x3b (cpu-model cpu)))
              "lujiazui"
-             (if-flags ("avx512" => "knl")
-                       ("adx" => "broadwell")
-                       ("avx2" => "haswell")
-                       ;; TODO: tigerlake, cooperlake, etc.
-                       ("avx" => "sandybridge")
-                       ("sse4_2" "gfni" => "tremont")
-                       ("sse4_2" "sgx" => "goldmont-plus")
-                       ("sse4_2" "xsave" => "goldmont")
-                       ("sse4_2" "movbe" => "silvermont")
-                       ("sse4_2" => "nehalem")
-                       ("ssse3" "movbe" => "bonnell")
-                       ("ssse3" "sse3" "longmode" => "nocona")
-                       ("ssse3" "sse3" "lm" => "nocona")
-                       ("ssse3" "sse3" => "prescott")
-                       ("ssse3" => "core2"))))
+             (cpu->generic-architecture cpu)))
 
          ;; TODO: Recognize CENTAUR/CYRIX/NSC?
 
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH 4/8] guix: cpu: Refactor cpu->gcc-architecture.

[Efraim Flashner]

* guix/cpu.scm (cpu->gcc-architecture): Refactor to wrap all the x86_64
options inside a common letrec-syntax.
---
 guix/cpu.scm | 59 +++++++++++++++++-----------------------------------
 1 file changed, 19 insertions(+), 40 deletions(-)

diff --git a/guix/cpu.scm b/guix/cpu.scm
index e500664503..ddfa4f20bb 100644
--- a/guix/cpu.scm
+++ b/guix/cpu.scm
@@ -115,19 +115,19 @@ (define (cpu->gcc-architecture cpu)
   (match (cpu-architecture cpu)
     ("x86_64"
      ;; Transcribed from GCC's 'host_detect_local_cpu' in driver-i386.cc.
-     (or (and (equal? "GenuineIntel" (cpu-vendor cpu))
-              (= 6 (cpu-family cpu))              ;the "Pentium Pro" family
-              (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 ...))))))
+     (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 ...))))))
 
+       (or (and (equal? "GenuineIntel" (cpu-vendor cpu))
+                (= 6 (cpu-family cpu))              ;the "Pentium Pro" family
                 (if-flags ("avx" "raoint" => "grandridge")
                           ("avx" "amx_fp16" => "graniterapids")
                           ("avx" "avxvnniint8" => "sierraforest")
@@ -153,20 +153,9 @@ (define (cpu->gcc-architecture cpu)
                           ("ssse3" "movbe" => "bonnell")
                           ("ssse3" => "core2")
                           ("longmode" => "x86-64")
-                          ("lm" => "x86-64"))))
-
-         (and (equal? "AuthenticAMD" (cpu-vendor cpu))
-              (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 ...))))))
+                          ("lm" => "x86-64")))
 
+           (and (equal? "AuthenticAMD" (cpu-vendor cpu))
                 (or (and (= 22 (cpu-family cpu))
                          (if-flags ("movbe" => "btver2")))
                     (and (= 6 (cpu-family cpu))
@@ -193,28 +182,18 @@ (define (cpu->gcc-architecture cpu)
                               ("lm" => "k8")
                               ("mmx" "3dnow" => "k6-3")
                               ("mmx" => "k6")
-                              (_ => "pentium")))))
+                              (_ => "pentium"))))
 
-         ;; 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 ...))))))
+           ;; Fallback case for non-Intel processors or for processors not
+           ;; recognized above.
            (if (and (= 7 (cpu-family cpu))
                     (= #x3b (cpu-model cpu)))
              "lujiazui"
-             (cpu->generic-architecture cpu)))
+             (cpu->generic-architecture cpu))
 
          ;; TODO: Recognize CENTAUR/CYRIX/NSC?
 
-         "x86_64"))
+         "x86_64")))
     ("aarch64"
      ;; Transcribed from GCC's list of aarch64 processors in aarch64-cores.def
      ;; What to do with big.LITTLE cores?
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH 5/8] gnu: go: Add CPU tuning targets.

[Efraim Flashner]

* gnu/packages/golang.scm (go-1.17)[properties]: New field.
(%go-1.17-arm-micro-architectures,
%go-1.17-powerpc64le-micro-architectures,
%go-1.18-x86_64-micro-architectures): New variables.
(go-1.18)[properties]: New field.
---
 gnu/packages/golang.scm | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/golang.scm b/gnu/packages/golang.scm
index d51c023808..c3ea30becc 100644
--- a/gnu/packages/golang.scm
+++ b/gnu/packages/golang.scm
@@ -628,6 +628,13 @@ (define-public go-1.16
      `(("go-fix-script-tests.patch" ,(search-patch "go-fix-script-tests.patch"))
        ,@(package-native-inputs go-1.14)))))
 
+;; https://github.com/golang/go/wiki/MinimumRequirements#microarchitecture-support
+(define %go-1.17-arm-micro-architectures
+  (list "armv5" "armv6" "armv7"))
+
+(define %go-1.17-powerpc64le-micro-architectures
+  (list "power8" "power9"))
+
 (define-public go-1.17
   (package
     (inherit go-1.16)
@@ -844,7 +851,14 @@ (define-public go-1.17
                   "README.md" "SECURITY.md"))))))))
     (inputs (if (not (or (target-arm?) (target-ppc64le?)))
               (alist-delete "gcc:lib" (package-inputs go-1.16))
-              (package-inputs go-1.16)))))
+              (package-inputs go-1.16)))
+    (properties
+     `((compiler-cpu-architectures
+         ("armhf" ,@%go-1.17-arm-micro-architectures)
+         ("powerpc64le" ,@%go-1.17-powerpc64le-micro-architectures))))))
+
+(define %go-1.18-x86_64-micro-architectures
+  (list "x86_64-v1" "x86_64-v2" "x86_64-v3" "x86_64-v4"))
 
 (define-public go-1.18
   (package
@@ -887,7 +901,12 @@ (define-public go-1.18
                            "ldflags, err := setextld(ldflags, compiler)\n"
                            "ldflags = append(ldflags, \"-r\")\n"
                            "ldflags = append(ldflags, \"" gcclib "\")\n")))))))
-               '())))))))
+               '())))))
+    (properties
+     `((compiler-cpu-architectures
+         ("armhf" ,@%go-1.17-arm-micro-architectures)
+         ("powerpc64le" ,@%go-1.17-powerpc64le-micro-architectures)
+         ("x86_64" ,@%go-1.18-x86_64-micro-architectures))))))
 
 (define-public go-1.19
   (package
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH 6/8] transformations: Allow tuning go packages.

[Efraim Flashner]

* guix/transformations.scm (tuned-package)[build-system]: Don't replace
the build-system if inheriting from the go-build-system.
[arguments]: If using the go-build-system add a phase to set the
micro-architecture for Go.
---
 guix/transformations.scm | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/guix/transformations.scm b/guix/transformations.scm
index a289f81219..47eb4b0515 100644
--- a/guix/transformations.scm
+++ b/guix/transformations.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2016-2023 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2021 Marius Bakke <marius@gnu.org>
 ;;; Copyright © 2023 Sarthak Shah <shahsarthakw@gmail.com>
+;;; Copyright © 2023 Efraim Flashner <efraim@flashner.co.il>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -570,14 +571,42 @@ (define (tuned-package p micro-architecture)
   (package
     (inherit p)
     (build-system
-      (build-system-with-tuning-compiler (package-build-system p)
-                                         micro-architecture))
+      ;; The go compiler doesn't need to be wrapped.
+      ;; XXX: This is where we check for supported micro-architectures.
+      (if (eq? (build-system-name (package-build-system p))
+               'go)
+        (package-build-system p)
+        (build-system-with-tuning-compiler (package-build-system p)
+                                           micro-architecture)))
     (arguments
      ;; The machine building this package may or may not be able to run code
      ;; for MICRO-ARCHITECTURE.  Because of that, skip tests; they are run for
      ;; the "baseline" variant anyway.
      (substitute-keyword-arguments (package-arguments p)
-       ((#:tests? _ #f) #f)))
+       ((#:tests? _ #f) #f)
+       ;; We add the tuning parameter after the default GO flags are set.
+       ((#:phases phases '%standard-phases)
+        (if (eq? (build-system-name (package-build-system p))
+                 'go)
+          #~(modify-phases #$phases
+              (add-after 'setup-go-environment 'set-microarchitecture
+                (lambda _
+                  (let ((microarch #$micro-architecture))
+                    (cond
+                      ((string-prefix? "arm" microarch)
+                       (setenv "GOARM" (string-take-right microarch 1))
+                       (format #t "Setting GOARM to ~s."
+                               (getenv "GOARM")))
+                      ((string-prefix? "powerpc" microarch)
+                       (setenv "GOPPC64" microarch)
+                       (format #t "Setting GOPPC64 to ~s."
+                               (getenv "GOPPC64")))
+                      ((string-prefix? "x86_64" microarch)
+                       (setenv "GOAMD" (string-take-right microarch 2))
+                       (format #t "Setting GOAMD to ~s.\n"
+                               (getenv "GOAMD")))
+                      (else #t))))))
+          phases))))
 
     (properties
      `((cpu-tuning . ,micro-architecture)
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH 7/8] guix: cpu: Add gcc-architecture->generic-architecture mapping.

[Efraim Flashner]

* guix/cpu.scm (gcc-architecture->generic-architecture): New variable.
---
 guix/cpu.scm | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/guix/cpu.scm b/guix/cpu.scm
index ddfa4f20bb..af26e2280b 100644
--- a/guix/cpu.scm
+++ b/guix/cpu.scm
@@ -33,7 +33,8 @@ (define-module (guix cpu)
             cpu-flags
 
             cpu->gcc-architecture
-            cpu->generic-architecture))
+            cpu->generic-architecture
+            gcc-architecture->generic-architecture))
 
 ;;; Commentary:
 ;;;
@@ -284,3 +285,21 @@ (define (cpu->generic-architecture cpu)
     (architecture
      ;; TODO: More architectures
      architecture)))
+
+(define (gcc-architecture->generic-architecture gcc-architecture)
+  "Return a generalized micro-architecture, using an inexact matching to provide
+'good enough' optimizations."
+  (match gcc-architecture
+    ((or "grandridge" "graniterapids" "sierraforest" "tigerlake"
+         "sapphirerapids" "cooperlake" "icelake-server" "icelake-client"
+         "cannonlake" "knm" "knl" "skylake-avx512" "alderlake" "skylake"
+         "broadwell" "haswell"
+         "znver4" "znver3" "znver2" "znver1" "bdver4")
+     "x86_64-v3")
+    ((or "sandybridge" "tremont" "goldmont-plus" "goldmont" "silvermont"
+         "nehalem" "bonnell" "core2"
+         "btver2" "athalon" "k8-sse3" "k8" "bdver3" "bdver2" "bdver1" "btver1"
+         "amdfam10"
+         "lujiazui" "x86-64")
+     "x86_64-v1")
+    (_ gcc-architecture)))
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH 8/8] transformations: Allow autotuning for go packages.

[Efraim Flashner]

* guix/transformations.scm (tuned-package)[build-system]: When the
inherited package uses the go-build-system, use a generic
micro-architecture.
[arguments]: Adjust the 'set-microarchitecture phase for changes in the
code.
---
 guix/transformations.scm | 50 ++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 23 deletions(-)

diff --git a/guix/transformations.scm b/guix/transformations.scm
index 47eb4b0515..122b0e74aa 100644
--- a/guix/transformations.scm
+++ b/guix/transformations.scm
@@ -33,7 +33,9 @@ (define-module (guix transformations)
   #:autoload   (guix upstream) (package-latest-release
                                 upstream-source-version
                                 upstream-source-signature-urls)
-  #:autoload   (guix cpu) (current-cpu cpu->gcc-architecture)
+  #:autoload   (guix cpu) (current-cpu
+                           cpu->gcc-architecture
+                           gcc-architecture->generic-architecture)
   #:use-module (guix utils)
   #:use-module (guix memoization)
   #:use-module (guix gexp)
@@ -568,16 +570,19 @@ (define (build-system-with-tuning-compiler bs micro-architecture)
 
 (define (tuned-package p micro-architecture)
   "Return package P tuned for MICRO-ARCHITECTURE."
+  (let ((generic-micro-arch (gcc-architecture->generic-architecture
+                              micro-architecture)))
   (package
     (inherit p)
     (build-system
-      ;; The go compiler doesn't need to be wrapped.
-      ;; XXX: This is where we check for supported micro-architectures.
-      (if (eq? (build-system-name (package-build-system p))
-               'go)
+      ;; The go compiler doesn't need to be wrapped but we want the error
+      ;; checking to make sure we're targeting a recognized optimization target.
+      (build-system-with-tuning-compiler
         (package-build-system p)
-        (build-system-with-tuning-compiler (package-build-system p)
-                                           micro-architecture)))
+        (if (eq? (build-system-name (package-build-system p))
+                 'go)
+          generic-micro-arch
+          micro-architecture)))
     (arguments
      ;; The machine building this package may or may not be able to run code
      ;; for MICRO-ARCHITECTURE.  Because of that, skip tests; they are run for
@@ -591,21 +596,20 @@ (define (tuned-package p micro-architecture)
           #~(modify-phases #$phases
               (add-after 'setup-go-environment 'set-microarchitecture
                 (lambda _
-                  (let ((microarch #$micro-architecture))
-                    (cond
-                      ((string-prefix? "arm" microarch)
-                       (setenv "GOARM" (string-take-right microarch 1))
-                       (format #t "Setting GOARM to ~s."
-                               (getenv "GOARM")))
-                      ((string-prefix? "powerpc" microarch)
-                       (setenv "GOPPC64" microarch)
-                       (format #t "Setting GOPPC64 to ~s."
-                               (getenv "GOPPC64")))
-                      ((string-prefix? "x86_64" microarch)
-                       (setenv "GOAMD" (string-take-right microarch 2))
-                       (format #t "Setting GOAMD to ~s.\n"
-                               (getenv "GOAMD")))
-                      (else #t))))))
+                  (cond
+                    ((string-prefix? "arm" #$generic-micro-arch)
+                     (setenv "GOARM" (string-take-right #$generic-micro-arch 1))
+                     (format #t "Setting GOARM to ~s."
+                             (getenv "GOARM")))
+                    ((string-prefix? "powerpc" #$generic-micro-arch)
+                     (setenv "GOPPC64" #$generic-micro-arch)
+                     (format #t "Setting GOPPC64 to ~s."
+                             (getenv "GOPPC64")))
+                    ((string-prefix? "x86_64" #$generic-micro-arch)
+                     (setenv "GOAMD" (string-take-right #$generic-micro-arch 2))
+                     (format #t "Setting GOAMD to ~s.\n"
+                             (getenv "GOAMD")))
+                    (else #t)))))
           phases))))
 
     (properties
@@ -613,7 +617,7 @@ (define (tuned-package p micro-architecture)
 
        ;; Remove the 'tunable?' property so that 'package-tuning' does not
        ;; call 'tuned-package' again on this one.
-       ,@(alist-delete 'tunable? (package-properties p))))))
+       ,@(alist-delete 'tunable? (package-properties p)))))))
 
 (define (tunable-package? package)
   "Return true if package PACKAGE is \"tunable\"--i.e., if tuning it for the
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH 0/8] More package tuning

[Ludovic Courtès]

Hello Efraim,

Efraim Flashner <efraim@flashner.co.il> skribis:

> with gcc-11, gcc gained support for using -march=x86_64-v{1,2,3,4},
> which I'm calling 'generic options,' as opposed to the more targeted
> tuning we have with specific architectures.

I don’t think these x86_64 psABI “architecture levels” should be treated
specially:

  • From the point of view of ‘--tune’, they’re just another value that
    may be passed to ‘-march’.

  • My understanding is that those levels don’t match reality: as
    discussed in the original ‘--tune’ patch¹, CPUs actually produced
    don’t follow a pattern of strictly including features of one set.
    They’re really just a simplification to get more memorizable names,
    but it’s hard to tell whether a given CPU really covers the set of
    features of a given level.

Overall, my take on this would be to add supported levels to
‘%gcc-11-x86_64-micro-architectures’ & co., without going further.

WDYT?

¹ https://issues.guix.gnu.org/52283#0-lineno48

[...]

> go cpu tuning targets: I mostly used the chart¹ on the go website, and I
> also checked the source code for go-1.18. I put in arm{5,6,7} as arm and
> not armhf since armhf only works with armv7 and with go programs, since
> they're statically linked, they can just be copied to other machines.

Now if Go uses those names, (guix cpu) can provide helpers.

Thanks,
Ludo’.

[bug#64188] [PATCH 0/8] More package tuning

[Ludovic Courtès]

Efraim Flashner <efraim@flashner.co.il> skribis:

> * gnu/packages/gcc.scm (%gcc-11-x86_64-micro-architectures): Add entries
> for x86_64-v{1,2,3,4}.

[...]

> +            "x86_64-v1" "x86_64-v2" "x86_64-v3" "x86_64-v4")))  ; Generic

Nitpick: I’d call it “psABI micro-architecture levels” because I think
that’s what the SysV psABI document calls them.

Otherwise LGTM!

Ludo’.

[bug#64188] [PATCH 0/8] More package tuning

[Ludovic Courtès]

Efraim Flashner <efraim@flashner.co.il> skribis:

> * guix/transformations.scm (tuned-package)[build-system]: Don't replace
> the build-system if inheriting from the go-build-system.
> [arguments]: If using the go-build-system add a phase to set the
> micro-architecture for Go.

[...]

> @@ -570,14 +571,42 @@ (define (tuned-package p micro-architecture)
>    (package
>      (inherit p)
>      (build-system
> -      (build-system-with-tuning-compiler (package-build-system p)
> -                                         micro-architecture))
> +      ;; The go compiler doesn't need to be wrapped.
> +      ;; XXX: This is where we check for supported micro-architectures.
> +      (if (eq? (build-system-name (package-build-system p))
> +               'go)
> +        (package-build-system p)
> +        (build-system-with-tuning-compiler (package-build-system p)
> +                                           micro-architecture)))
>      (arguments
>       ;; The machine building this package may or may not be able to run code
>       ;; for MICRO-ARCHITECTURE.  Because of that, skip tests; they are run for
>       ;; the "baseline" variant anyway.
>       (substitute-keyword-arguments (package-arguments p)
> -       ((#:tests? _ #f) #f)))
> +       ((#:tests? _ #f) #f)
> +       ;; We add the tuning parameter after the default GO flags are set.
> +       ((#:phases phases '%standard-phases)
> +        (if (eq? (build-system-name (package-build-system p))
> +                 'go)
> +          #~(modify-phases #$phases
> +              (add-after 'setup-go-environment 'set-microarchitecture

Can we use the same approach as before with
‘build-system-with-tuning-compiler’?  That seems more robust to me and
it’d be more consistent.

Ludo’.

[bug#64188] [PATCH 0/8] More package tuning

[Ludovic Courtès]

Efraim Flashner <efraim@flashner.co.il> skribis:

> * guix/cpu.scm (gcc-architecture->generic-architecture): New variable.

[...]

> +(define (gcc-architecture->generic-architecture gcc-architecture)
> +  "Return a generalized micro-architecture, using an inexact matching to provide
> +'good enough' optimizations."
> +  (match gcc-architecture

Let’s call it ‘gcc-architecture->micro-architecture-level’, with a
docstring that makes it’s clear that this is about x86_64
micro-architecture levels.

Please also add a comment stating how to figure out what goes where so
we can more easily keep it up-to-date.

I guess this procedure is good enough for Go support, right?

Thanks,
Ludo’.

[bug#64188] [PATCH 0/8] More package tuning

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 2754 bytes --]

On Sun, Jun 25, 2023 at 10:52:14PM +0200, Ludovic Courtès wrote:
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > * guix/transformations.scm (tuned-package)[build-system]: Don't replace
> > the build-system if inheriting from the go-build-system.
> > [arguments]: If using the go-build-system add a phase to set the
> > micro-architecture for Go.
> 
> [...]
> 
> > @@ -570,14 +571,42 @@ (define (tuned-package p micro-architecture)
> >    (package
> >      (inherit p)
> >      (build-system
> > -      (build-system-with-tuning-compiler (package-build-system p)
> > -                                         micro-architecture))
> > +      ;; The go compiler doesn't need to be wrapped.
> > +      ;; XXX: This is where we check for supported micro-architectures.
> > +      (if (eq? (build-system-name (package-build-system p))
> > +               'go)
> > +        (package-build-system p)
> > +        (build-system-with-tuning-compiler (package-build-system p)
> > +                                           micro-architecture)))
> >      (arguments
> >       ;; The machine building this package may or may not be able to run code
> >       ;; for MICRO-ARCHITECTURE.  Because of that, skip tests; they are run for
> >       ;; the "baseline" variant anyway.
> >       (substitute-keyword-arguments (package-arguments p)
> > -       ((#:tests? _ #f) #f)))
> > +       ((#:tests? _ #f) #f)
> > +       ;; We add the tuning parameter after the default GO flags are set.
> > +       ((#:phases phases '%standard-phases)
> > +        (if (eq? (build-system-name (package-build-system p))
> > +                 'go)
> > +          #~(modify-phases #$phases
> > +              (add-after 'setup-go-environment 'set-microarchitecture
> 
> Can we use the same approach as before with
> ‘build-system-with-tuning-compiler’?  That seems more robust to me and
> it’d be more consistent.

I'll look to see how to move this into
build-system-with-tuning-compiler. I think I previously got hung-up on
the tuning-compiler function which wouldn't work for go. At first glance
it looks like I could just lift-and-shift the extra phase over. I'll
just have to make sure that either it's only for the go-build-system or
occurs sometime after the setup-go-environment phase but before/after a
phase which exists in all build systems. Then it's the toss-up between
leaving it as-is and only occurring with the go-build-system or also
occurring on any tuned package which also has go code.

-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#64188] [PATCH 0/8] More package tuning

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 1830 bytes --]

On Sun, Jun 25, 2023 at 10:54:38PM +0200, Ludovic Courtès wrote:
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > * guix/cpu.scm (gcc-architecture->generic-architecture): New variable.
> 
> [...]
> 
> > +(define (gcc-architecture->generic-architecture gcc-architecture)
> > +  "Return a generalized micro-architecture, using an inexact matching to provide
> > +'good enough' optimizations."
> > +  (match gcc-architecture
> 
> Let’s call it ‘gcc-architecture->micro-architecture-level’, with a
> docstring that makes it’s clear that this is about x86_64
> micro-architecture levels.

Sounds good to me. We can change the docstring if we end up adding
support for more architectures (ie ppc64le).

> Please also add a comment stating how to figure out what goes where so
> we can more easily keep it up-to-date.
> 
> I guess this procedure is good enough for Go support, right?

It would be better if we could populate x86_64-v4 and x86_64-v2, but I
really didn't feel like I could comfortably draw the line anywhere and
be sure I had the correct CPUs. If someone comes along and says they're
sure their CPU should be in -v2 or -v4 then I/someone can check the gcc
sources to make sure that's the case.

I'm pretty sure there's other compilers out there that also use the
psABI for tuning, my first look would be GHC. From my testing it does
work for Go, and since we're using an official definition for the psABIs
and not just Go's definitions I'm pretty confident it'll be useful for
something else, somewhere.

> Thanks,
> Ludo’.

Thanks for your review!

-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#64188] [PATCH 0/8] More package tuning

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 2510 bytes --]

On Sun, Jun 25, 2023 at 10:47:42PM +0200, Ludovic Courtès wrote:
> Hello Efraim,
> 
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > with gcc-11, gcc gained support for using -march=x86_64-v{1,2,3,4},
> > which I'm calling 'generic options,' as opposed to the more targeted
> > tuning we have with specific architectures.
> 
> I don’t think these x86_64 psABI “architecture levels” should be treated
> specially:
> 
>   • From the point of view of ‘--tune’, they’re just another value that
>     may be passed to ‘-march’.
> 
>   • My understanding is that those levels don’t match reality: as
>     discussed in the original ‘--tune’ patch¹, CPUs actually produced
>     don’t follow a pattern of strictly including features of one set.
>     They’re really just a simplification to get more memorizable names,
>     but it’s hard to tell whether a given CPU really covers the set of
>     features of a given level.

They're also useful for glibc-hwcaps, so that we could build each
package multiple times and install libraries built for the psABI levels
into $prefix/lib/glibc-hwcaps/x86-64-v[234]/, but I agree that, for our
uses so far, they're not really useful.

> Overall, my take on this would be to add supported levels to
> ‘%gcc-11-x86_64-micro-architectures’ & co., without going further.
> 
> WDYT?

I could see keeping the code from cpu->generic-architecture (renamed
cpu->psABI) either as a non-exported function or simply moved into the
fallback for x86_64.

> ¹ https://issues.guix.gnu.org/52283#0-lineno48
> 
> [...]
> 
> > go cpu tuning targets: I mostly used the chart¹ on the go website, and I
> > also checked the source code for go-1.18. I put in arm{5,6,7} as arm and
> > not armhf since armhf only works with armv7 and with go programs, since
> > they're statically linked, they can just be copied to other machines.
> 
> Now if Go uses those names, (guix cpu) can provide helpers.

Go also uses power8 and power9 as PPC64(le) options, so that's also a
possible use-case I was trying to also prepare for. That was my plan for
the gcc-architecture->generic-architecture function, to allow for --tune
to work without needing to pass different values to different packages.

-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#64188] [PATCH v2 0/7] More package tuning

[Efraim Flashner]

I hope I've addressed the comments with this patch series. I've changed
the language to use psabi or to refer to 'generalized optimizations'.
The go adaptations for tuning are now part of
build-system-with-tuning-compiler, with an adjustment made to (also)
check the psabi listing when it comes to the go compiler.


Efraim Flashner (7):
  gnu: %gcc-11-x86_64-micro-architectures: Add psabi entries.
  guix: cpu: Add generalized CPU matching.
  guix: cpu: Rewrite fallback for x86_64 cpu->gcc-architecture.
  guix: cpu: Refactor cpu->gcc-architecture.
  guix: cpu: Add gcc-architecture->micro-architecture-level mapping.
  gnu: go: Add CPU tuning targets.
  transformations: Allow tuning go packages.

 gnu/packages/gcc.scm     |   5 +-
 gnu/packages/golang.scm  |  23 ++++++-
 guix/cpu.scm             | 130 ++++++++++++++++++++++-----------------
 guix/transformations.scm |  37 +++++++++--
 4 files changed, 133 insertions(+), 62 deletions(-)


base-commit: e85593b36b3874227ba812f47113441928c0f0c1
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH v2 1/7] gnu: %gcc-11-x86_64-micro-architectures: Add psabi entries.

[Efraim Flashner]

* gnu/packages/gcc.scm (%gcc-11-x86_64-micro-architectures): Add entries
for x86_64-v{1,2,3,4}.
---
 gnu/packages/gcc.scm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/gcc.scm b/gnu/packages/gcc.scm
index 1b444c2b02..862fdd79c3 100644
--- a/gnu/packages/gcc.scm
+++ b/gnu/packages/gcc.scm
@@ -614,7 +614,10 @@ (define %gcc-11-x86_64-micro-architectures
   (append %gcc-10-x86_64-micro-architectures
           '("sapphirerapids" "alterlake" "rocketlake" ;Intel
 
-            "btver1" "btver2")))                  ;AMD
+            "btver1" "btver2"                     ;AMD
+
+            ;; psABI micro-architecture levels
+            "x86_64-v1" "x86_64-v2" "x86_64-v3" "x86_64-v4")))
 
 ;; Suitable '-march' values for GCC 12.
 (define %gcc-12-aarch64-micro-architectures
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH v2 2/7] guix: cpu: Add generalized CPU matching.

[Efraim Flashner]

* guix/cpu.scm (cpu->micro-architecture-level): New variable.
---
 guix/cpu.scm | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/guix/cpu.scm b/guix/cpu.scm
index 45e1abeed7..3fc12327ed 100644
--- a/guix/cpu.scm
+++ b/guix/cpu.scm
@@ -285,3 +285,36 @@ (define (cpu->gcc-architecture cpu)
     (architecture
      ;; TODO: More architectures
      architecture)))
+
+(define (cpu->micro-architecture-level cpu)
+  "Return a micro-architecture name, suitable for generalized optimizations that
+correspond roughly to CPU, a record as returned by 'current-cpu'."
+  (match (cpu-architecture cpu)
+    ("x86_64"
+     (or (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
+             ;; https://gitlab.com/x86-psABIs/x86-64-ABI/-/blob/master/x86-64-ABI/low-level-sys-info.tex
+             ;; v4: AVX512F, AVX512BW, AVX512CD, AVX512DQ, AVX512VL
+             ;; v3: AVX, AVX2, BMI1, BMI2, F16C, FMA, LZCNT, MOVBE, OSXSAVE
+             ;; v2: CMPXCHG16B, LAHF, SAHF, POPCNT, SSE3, SSE4.1, SSE4.2, SSSE3
+             ("avx512f" "avx512bw" "abx512cd" "abx512dq" "avx512vl"
+              "avx" "avx2" "bmi1" "bmi2" "f16c" "fma" "movbe"
+              "popcnt" "sse3" "sse4_1" "sse4_2" "ssse3" => "x86_64-v4")
+             ("avx" "avx2" "bmi1" "bmi2" "f16c" "fma" "movbe"
+              "popcnt" "sse3" "sse4_1" "sse4_2" "ssse3" => "x86_64-v3")
+             ("popcnt" "sse3" "sse4_1" "sse4_2" "ssse3" => "x86_64-v2")
+             (_ => "x86_64-v1")))
+         "x86_64-v1"))
+    (architecture
+     ;; TODO: More architectures
+     architecture)))
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH v2 3/7] guix: cpu: Rewrite fallback for x86_64 cpu->gcc-architecture.

[Efraim Flashner]

* guix/cpu.scm (cpu->gcc-architecture): Adjust the fallback case to use
cpu->micro-architecture-level.
---
 guix/cpu.scm | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/guix/cpu.scm b/guix/cpu.scm
index 3fc12327ed..1a75b8ac62 100644
--- a/guix/cpu.scm
+++ b/guix/cpu.scm
@@ -209,21 +209,7 @@ (define (cpu->gcc-architecture cpu)
            (if (and (= 7 (cpu-family cpu))
                     (= #x3b (cpu-model cpu)))
              "lujiazui"
-             (if-flags ("avx512" => "knl")
-                       ("adx" => "broadwell")
-                       ("avx2" => "haswell")
-                       ;; TODO: tigerlake, cooperlake, etc.
-                       ("avx" => "sandybridge")
-                       ("sse4_2" "gfni" => "tremont")
-                       ("sse4_2" "sgx" => "goldmont-plus")
-                       ("sse4_2" "xsave" => "goldmont")
-                       ("sse4_2" "movbe" => "silvermont")
-                       ("sse4_2" => "nehalem")
-                       ("ssse3" "movbe" => "bonnell")
-                       ("ssse3" "sse3" "longmode" => "nocona")
-                       ("ssse3" "sse3" "lm" => "nocona")
-                       ("ssse3" "sse3" => "prescott")
-                       ("ssse3" => "core2"))))
+             (cpu->micro-architecture-level cpu))
 
          ;; TODO: Recognize CENTAUR/CYRIX/NSC?
 
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH v2 4/7] guix: cpu: Refactor cpu->gcc-architecture.

[Efraim Flashner]

* guix/cpu.scm (cpu->gcc-architecture): Refactor to wrap all the x86_64
options inside a common letrec-syntax.
---
 guix/cpu.scm | 57 +++++++++++++++++-----------------------------------
 1 file changed, 18 insertions(+), 39 deletions(-)

diff --git a/guix/cpu.scm b/guix/cpu.scm
index 1a75b8ac62..30cd860a19 100644
--- a/guix/cpu.scm
+++ b/guix/cpu.scm
@@ -114,19 +114,19 @@ (define (cpu->gcc-architecture cpu)
   (match (cpu-architecture cpu)
     ("x86_64"
      ;; Transcribed from GCC's 'host_detect_local_cpu' in driver-i386.cc.
-     (or (and (equal? "GenuineIntel" (cpu-vendor cpu))
-              (= 6 (cpu-family cpu))              ;the "Pentium Pro" family
-              (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 ...))))))
+     (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 ...))))))
 
+       (or (and (equal? "GenuineIntel" (cpu-vendor cpu))
+                (= 6 (cpu-family cpu))              ;the "Pentium Pro" family
                 (if-flags ("avx" "raoint" => "grandridge")
                           ("avx" "amx_fp16" => "graniterapids")
                           ("avx" "avxvnniint8" => "sierraforest")
@@ -152,20 +152,9 @@ (define (cpu->gcc-architecture cpu)
                           ("ssse3" "movbe" => "bonnell")
                           ("ssse3" => "core2")
                           ("longmode" => "x86-64")
-                          ("lm" => "x86-64"))))
-
-         (and (equal? "AuthenticAMD" (cpu-vendor cpu))
-              (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 ...))))))
+                          ("lm" => "x86-64")))
 
+           (and (equal? "AuthenticAMD" (cpu-vendor cpu))
                 (or (and (= 22 (cpu-family cpu))
                          (if-flags ("movbe" => "btver2")))
                     (and (= 6 (cpu-family cpu))
@@ -192,20 +181,10 @@ (define (cpu->gcc-architecture cpu)
                               ("lm" => "k8")
                               ("mmx" "3dnow" => "k6-3")
                               ("mmx" => "k6")
-                              (_ => "pentium")))))
+                              (_ => "pentium"))))
 
-         ;; 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 ...))))))
+           ;; Fallback case for non-Intel processors or for processors not
+           ;; recognized above.
            (if (and (= 7 (cpu-family cpu))
                     (= #x3b (cpu-model cpu)))
              "lujiazui"
@@ -213,7 +192,7 @@ (define (cpu->gcc-architecture cpu)
 
          ;; TODO: Recognize CENTAUR/CYRIX/NSC?
 
-         "x86_64"))
+         "x86_64")))
     ("aarch64"
      ;; Transcribed from GCC's list of aarch64 processors in aarch64-cores.def
      ;; What to do with big.LITTLE cores?
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH v2 5/7] guix: cpu: Add gcc-architecture->micro-architecture-level mapping.

[Efraim Flashner]

* guix/cpu.scm (gcc-architecture->micro-architecture-level): New
variable.
---
 guix/cpu.scm | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/guix/cpu.scm b/guix/cpu.scm
index 30cd860a19..29ad883584 100644
--- a/guix/cpu.scm
+++ b/guix/cpu.scm
@@ -32,7 +32,8 @@ (define-module (guix cpu)
             cpu-model
             cpu-flags
 
-            cpu->gcc-architecture))
+            cpu->gcc-architecture
+            gcc-architecture->micro-architecture-level))
 
 ;;; Commentary:
 ;;;
@@ -283,3 +284,24 @@ (define (cpu->micro-architecture-level cpu)
     (architecture
      ;; TODO: More architectures
      architecture)))
+
+(define (gcc-architecture->micro-architecture-level gcc-architecture)
+  "Return a matching psABI micro-architecture, allowing optimizations for x86_64
+CPUs for compilers which don't allow for more focused optimizing."
+  ;; Matching gcc-architectures isn't an easy task, with the rule-of-thumb being
+  ;; 'Haswell and higher' qualify for x86_64-v3.
+  ;; https://gitlab.com/x86-psABIs/x86-64-ABI/-/blob/master/x86-64-ABI/low-level-sys-info.tex
+  (match gcc-architecture
+    ((or "grandridge" "graniterapids" "sierraforest" "tigerlake"
+         "sapphirerapids" "cooperlake" "icelake-server" "icelake-client"
+         "cannonlake" "knm" "knl" "skylake-avx512" "alderlake" "skylake"
+         "broadwell" "haswell"
+         "znver4" "znver3" "znver2" "znver1" "bdver4")
+     "x86_64-v3")
+    ((or "sandybridge" "tremont" "goldmont-plus" "goldmont" "silvermont"
+         "nehalem" "bonnell" "core2"
+         "btver2" "athalon" "k8-sse3" "k8" "bdver3" "bdver2" "bdver1" "btver1"
+         "amdfam10"
+         "lujiazui" "x86-64")
+     "x86_64-v1")
+    (_ gcc-architecture)))
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH v2 6/7] gnu: go: Add CPU tuning targets.

[Efraim Flashner]

* gnu/packages/golang.scm (go-1.17)[properties]: New field.
(%go-1.17-arm-micro-architectures,
%go-1.17-powerpc64le-micro-architectures,
%go-1.18-x86_64-micro-architectures): New variables.
(go-1.18)[properties]: New field.
---
 gnu/packages/golang.scm | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/golang.scm b/gnu/packages/golang.scm
index e4c4cb299b..55cc40dda5 100644
--- a/gnu/packages/golang.scm
+++ b/gnu/packages/golang.scm
@@ -628,6 +628,13 @@ (define-public go-1.16
      `(("go-fix-script-tests.patch" ,(search-patch "go-fix-script-tests.patch"))
        ,@(package-native-inputs go-1.14)))))
 
+;; https://github.com/golang/go/wiki/MinimumRequirements#microarchitecture-support
+(define %go-1.17-arm-micro-architectures
+  (list "armv5" "armv6" "armv7"))
+
+(define %go-1.17-powerpc64le-micro-architectures
+  (list "power8" "power9"))
+
 (define-public go-1.17
   (package
     (inherit go-1.16)
@@ -844,7 +851,14 @@ (define-public go-1.17
                   "README.md" "SECURITY.md"))))))))
     (inputs (if (not (or (target-arm?) (target-ppc64le?)))
               (alist-delete "gcc:lib" (package-inputs go-1.16))
-              (package-inputs go-1.16)))))
+              (package-inputs go-1.16)))
+    (properties
+     `((compiler-cpu-architectures
+         ("armhf" ,@%go-1.17-arm-micro-architectures)
+         ("powerpc64le" ,@%go-1.17-powerpc64le-micro-architectures))))))
+
+(define %go-1.18-x86_64-micro-architectures
+  (list "x86_64-v1" "x86_64-v2" "x86_64-v3" "x86_64-v4"))
 
 (define-public go-1.18
   (package
@@ -887,7 +901,12 @@ (define-public go-1.18
                            "ldflags, err := setextld(ldflags, compiler)\n"
                            "ldflags = append(ldflags, \"-r\")\n"
                            "ldflags = append(ldflags, \"" gcclib "\")\n")))))))
-               '())))))))
+               '())))))
+    (properties
+     `((compiler-cpu-architectures
+         ("armhf" ,@%go-1.17-arm-micro-architectures)
+         ("powerpc64le" ,@%go-1.17-powerpc64le-micro-architectures)
+         ("x86_64" ,@%go-1.18-x86_64-micro-architectures))))))
 
 (define-public go-1.19
   (package
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH v2 7/7] transformations: Allow tuning go packages.

[Efraim Flashner]

* guix/transformations.scm (build-system-with-tuning-compiler): When
checking if a microarchitecture is supported by the compiler, also check
if it is a go compiler which supports that psabi.  Add a phase after
'setup-go-environment to set the go microarchitecture.
---
 guix/transformations.scm | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/guix/transformations.scm b/guix/transformations.scm
index a289f81219..92d9c89c0e 100644
--- a/guix/transformations.scm
+++ b/guix/transformations.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2016-2023 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2021 Marius Bakke <marius@gnu.org>
 ;;; Copyright © 2023 Sarthak Shah <shahsarthakw@gmail.com>
+;;; Copyright © 2023 Efraim Flashner <efraim@flashner.co.il>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -32,7 +33,9 @@ (define-module (guix transformations)
   #:autoload   (guix upstream) (package-latest-release
                                 upstream-source-version
                                 upstream-source-signature-urls)
-  #:autoload   (guix cpu) (current-cpu cpu->gcc-architecture)
+  #:autoload   (guix cpu) (current-cpu
+                           cpu->gcc-architecture
+                           gcc-architecture->micro-architecture-level)
   #:use-module (guix utils)
   #:use-module (guix memoization)
   #:use-module (guix gexp)
@@ -516,7 +519,9 @@ (define (build-system-with-tuning-compiler bs micro-architecture)
                                                  'compiler-cpu-architectures)
                                       p))
                                 (_ #f))
-                              (bag-build-inputs lowered))))
+                              (bag-build-inputs lowered)))
+           (psabi        (gcc-architecture->micro-architecture-level
+                           micro-architecture)))
       (unless compiler
         (raise (formatted-message
                 (G_ "failed to determine which compiler is used"))))
@@ -528,8 +533,11 @@ (define (build-system-with-tuning-compiler bs micro-architecture)
                   (G_ "failed to determine whether ~a supports ~a")
                   (package-full-name compiler)
                   micro-architecture)))
-        (unless (member micro-architecture
-                        (or (assoc-ref lst architecture) '()))
+        (unless (or (member micro-architecture
+                            (or (assoc-ref lst architecture) '()))
+                    (and (string=? (package-name compiler) "go")
+                         (member psabi
+                                 (or (assoc-ref lst architecture) '()))))
           (raise
            (make-compound-condition
             (formatted-message
@@ -556,6 +564,27 @@ (define (build-system-with-tuning-compiler bs micro-architecture)
 
       (bag
         (inherit lowered)
+        (arguments
+          (substitute-keyword-arguments (bag-arguments lowered)
+          ;; We add the tuning parameter after the default GO flags are set.
+          ((#:phases phases '%standard-phases)
+             #~(modify-phases #$phases
+                 (add-after 'setup-go-environment 'set-microarchitecture
+                   (lambda _
+                     (cond
+                       ((string-prefix? "arm" #$psabi)
+                        (setenv "GOARM" (string-take-right #$psabi 1))
+                        (format #t "Setting GOARM to ~s."
+                                (getenv "GOARM")))
+                       ((string-prefix? "powerpc" #$psabi)
+                        (setenv "GOPPC64" #$psabi)
+                        (format #t "Setting GOPPC64 to ~s."
+                                (getenv "GOPPC64")))
+                       ((string-prefix? "x86_64" #$psabi)
+                        (setenv "GOAMD" (string-take-right #$psabi 2))
+                        (format #t "Setting GOAMD to ~s.\n"
+                                (getenv "GOAMD")))
+                       (else #t))))))))
         (build-inputs
          ;; Arrange so that the compiler wrapper comes first in $PATH.
          `(("tuning-compiler" ,(tuning-compiler micro-architecture))
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#64188] [PATCH 0/8] More package tuning

[Ludovic Courtès]

Hi Efraim,

Thanks for pushing this patch series past the finish line!

Efraim Flashner <efraim@flashner.co.il> skribis:

> I'll look to see how to move this into
> build-system-with-tuning-compiler. I think I previously got hung-up on
> the tuning-compiler function which wouldn't work for go. At first glance
> it looks like I could just lift-and-shift the extra phase over. I'll
> just have to make sure that either it's only for the go-build-system or
> occurs sometime after the setup-go-environment phase but before/after a
> phase which exists in all build systems. Then it's the toss-up between
> leaving it as-is and only occurring with the go-build-system or also
> occurring on any tuned package which also has go code.

It looks like we’re now adding the ‘set-microarchitecture’ phase
unconditionally, not just for go.  For example:

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix build --tune eigen-benchmarks --log-file
guix build: tuning eigen-benchmarks@3.4.0 for CPU skylake
https://ci.guix.gnu.org/log/djwka1jhzhk08yb23as83yk5hysn0pky-eigen-benchmarks-3.4.0
$ wget -qO- https://ci.guix.gnu.org/log/djwka1jhzhk08yb23as83yk5hysn0pky-eigen-benchmarks-3.4.0 |gunzip -c| grep -C3 set-micro
phase `reset-gzip-timestamps' succeeded after 0.0 seconds
starting phase `compress-documentation'
phase `compress-documentation' succeeded after 0.0 seconds
starting phase `set-microarchitecture'
Setting GOAMD to "v3".
phase `set-microarchitecture' succeeded after 0.0 seconds
@ build-succeeded /gnu/store/pdz0g9q2yd9i1jkbhk2rnbfa88ngvffw-eigen-benchmarks-3.4.0.drv -
--8<---------------cut here---------------end--------------->8---

What I had in mind was to have a procedure similar to ‘tuning-compiler’
that would return a wrapper around the “go” binary that would set
‘GOAMD’ (or similar).  That way the change would be well isolated.

Could you look into providing a patch for that?

Thanks in advance!

Ludo’.

[bug#64188] [PATCH 0/8] More package tuning

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 3253 bytes --]

On Thu, Jul 13, 2023 at 05:27:21PM +0200, Ludovic Courtès wrote:
> Hi Efraim,
> 
> Thanks for pushing this patch series past the finish line!
> 
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > I'll look to see how to move this into
> > build-system-with-tuning-compiler. I think I previously got hung-up on
> > the tuning-compiler function which wouldn't work for go. At first glance
> > it looks like I could just lift-and-shift the extra phase over. I'll
> > just have to make sure that either it's only for the go-build-system or
> > occurs sometime after the setup-go-environment phase but before/after a
> > phase which exists in all build systems. Then it's the toss-up between
> > leaving it as-is and only occurring with the go-build-system or also
> > occurring on any tuned package which also has go code.
> 
> It looks like we’re now adding the ‘set-microarchitecture’ phase
> unconditionally, not just for go.  For example:
> 
> --8<---------------cut here---------------start------------->8---
> $ ./pre-inst-env guix build --tune eigen-benchmarks --log-file
> guix build: tuning eigen-benchmarks@3.4.0 for CPU skylake
> https://ci.guix.gnu.org/log/djwka1jhzhk08yb23as83yk5hysn0pky-eigen-benchmarks-3.4.0
> $ wget -qO- https://ci.guix.gnu.org/log/djwka1jhzhk08yb23as83yk5hysn0pky-eigen-benchmarks-3.4.0 |gunzip -c| grep -C3 set-micro
> phase `reset-gzip-timestamps' succeeded after 0.0 seconds
> starting phase `compress-documentation'
> phase `compress-documentation' succeeded after 0.0 seconds
> starting phase `set-microarchitecture'
> Setting GOAMD to "v3".
> phase `set-microarchitecture' succeeded after 0.0 seconds
> @ build-succeeded /gnu/store/pdz0g9q2yd9i1jkbhk2rnbfa88ngvffw-eigen-benchmarks-3.4.0.drv -
> --8<---------------cut here---------------end--------------->8---
> 
> What I had in mind was to have a procedure similar to ‘tuning-compiler’
> that would return a wrapper around the “go” binary that would set
> ‘GOAMD’ (or similar).  That way the change would be well isolated.
> 
> Could you look into providing a patch for that?
> 
> Thanks in advance!
> 
> Ludo’.

That's actually really surprising to me. I thought that if you tried to
add a phase after a non-existent phase then it just wouldn't be added.

I tried just wrapping the call to the 'go' binary itself so that every
time 'go' was called it would also set the environment variable setting
the optimization level but I was having a hard time working that. While
experimenting I did change what I had written to check for the
'setup-go-environment phase, and if it existed to add the optimization
at the end of that phase.

I have the part with wrapping the go binary as a WIP, and when it's
ready I'll post both parts so we can choose which one seems better. I
like the idea of go being wrapped, it makes it easier to just add in the
optimizations whenever go is added to a package. On the other hand I
like the extra phase, since it's already done :)

-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#64188] [PATCH 0/8] More package tuning

[Ludovic Courtès]

Hi,

Efraim Flashner <efraim@flashner.co.il> skribis:

> On Thu, Jul 13, 2023 at 05:27:21PM +0200, Ludovic Courtès wrote:

[...]

>> It looks like we’re now adding the ‘set-microarchitecture’ phase
>> unconditionally, not just for go.  For example:
>> 
>> --8<---------------cut here---------------start------------->8---
>> $ ./pre-inst-env guix build --tune eigen-benchmarks --log-file
>> guix build: tuning eigen-benchmarks@3.4.0 for CPU skylake
>> https://ci.guix.gnu.org/log/djwka1jhzhk08yb23as83yk5hysn0pky-eigen-benchmarks-3.4.0
>> $ wget -qO- https://ci.guix.gnu.org/log/djwka1jhzhk08yb23as83yk5hysn0pky-eigen-benchmarks-3.4.0 |gunzip -c| grep -C3 set-micro
>> phase `reset-gzip-timestamps' succeeded after 0.0 seconds
>> starting phase `compress-documentation'
>> phase `compress-documentation' succeeded after 0.0 seconds
>> starting phase `set-microarchitecture'
>> Setting GOAMD to "v3".
>> phase `set-microarchitecture' succeeded after 0.0 seconds
>> @ build-succeeded /gnu/store/pdz0g9q2yd9i1jkbhk2rnbfa88ngvffw-eigen-benchmarks-3.4.0.drv -
>> --8<---------------cut here---------------end--------------->8---
>> 
>> What I had in mind was to have a procedure similar to ‘tuning-compiler’
>> that would return a wrapper around the “go” binary that would set
>> ‘GOAMD’ (or similar).  That way the change would be well isolated.
>> 
>> Could you look into providing a patch for that?
>> 
>> Thanks in advance!
>> 
>> Ludo’.
>
> That's actually really surprising to me. I thought that if you tried to
> add a phase after a non-existent phase then it just wouldn't be added.

Actually I thought so too.  :-)

But anyway, the point is that we’re modifying phases unconditionally
(whether or not this has an effect), and it would be nicer to avoid it
IMO.

> I tried just wrapping the call to the 'go' binary itself so that every
> time 'go' was called it would also set the environment variable setting
> the optimization level but I was having a hard time working that. While
> experimenting I did change what I had written to check for the
> 'setup-go-environment phase, and if it existed to add the optimization
> at the end of that phase.
>
> I have the part with wrapping the go binary as a WIP, and when it's
> ready I'll post both parts so we can choose which one seems better. I
> like the idea of go being wrapped, it makes it easier to just add in the
> optimizations whenever go is added to a package. On the other hand I
> like the extra phase, since it's already done :)

Not a valid argument! :-)  We can discuss the implementation on IRC if
you want.  It might be that we can slightly generalize ‘tuning-compiler’
so that it works for go (perhaps there’s an option like ‘-march’ that we
could use instead of setting ‘GOAMD’?).

Thanks in advance!

Ludo’.

[bug#64188] [PATCH 0/8] More package tuning

[Efraim Flashner]

[-- Attachment #1.1: Type: text/plain, Size: 3815 bytes --]

On Mon, Jul 17, 2023 at 05:41:35PM +0200, Ludovic Courtès wrote:
> Hi,
> 
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > On Thu, Jul 13, 2023 at 05:27:21PM +0200, Ludovic Courtès wrote:
> 
> [...]
> 
> >> It looks like we’re now adding the ‘set-microarchitecture’ phase
> >> unconditionally, not just for go.  For example:
> >> 
> >> --8<---------------cut here---------------start------------->8---
> >> $ ./pre-inst-env guix build --tune eigen-benchmarks --log-file
> >> guix build: tuning eigen-benchmarks@3.4.0 for CPU skylake
> >> https://ci.guix.gnu.org/log/djwka1jhzhk08yb23as83yk5hysn0pky-eigen-benchmarks-3.4.0
> >> $ wget -qO- https://ci.guix.gnu.org/log/djwka1jhzhk08yb23as83yk5hysn0pky-eigen-benchmarks-3.4.0 |gunzip -c| grep -C3 set-micro
> >> phase `reset-gzip-timestamps' succeeded after 0.0 seconds
> >> starting phase `compress-documentation'
> >> phase `compress-documentation' succeeded after 0.0 seconds
> >> starting phase `set-microarchitecture'
> >> Setting GOAMD to "v3".
> >> phase `set-microarchitecture' succeeded after 0.0 seconds
> >> @ build-succeeded /gnu/store/pdz0g9q2yd9i1jkbhk2rnbfa88ngvffw-eigen-benchmarks-3.4.0.drv -
> >> --8<---------------cut here---------------end--------------->8---
> >> 
> >> What I had in mind was to have a procedure similar to ‘tuning-compiler’
> >> that would return a wrapper around the “go” binary that would set
> >> ‘GOAMD’ (or similar).  That way the change would be well isolated.
> >> 
> >> Could you look into providing a patch for that?
> >> 
> >> Thanks in advance!
> >> 
> >> Ludo’.
> >
> > That's actually really surprising to me. I thought that if you tried to
> > add a phase after a non-existent phase then it just wouldn't be added.
> 
> Actually I thought so too.  :-)
> 
> But anyway, the point is that we’re modifying phases unconditionally
> (whether or not this has an effect), and it would be nicer to avoid it
> IMO.
> 
> > I tried just wrapping the call to the 'go' binary itself so that every
> > time 'go' was called it would also set the environment variable setting
> > the optimization level but I was having a hard time working that. While
> > experimenting I did change what I had written to check for the
> > 'setup-go-environment phase, and if it existed to add the optimization
> > at the end of that phase.
> >
> > I have the part with wrapping the go binary as a WIP, and when it's
> > ready I'll post both parts so we can choose which one seems better. I
> > like the idea of go being wrapped, it makes it easier to just add in the
> > optimizations whenever go is added to a package. On the other hand I
> > like the extra phase, since it's already done :)
> 
> Not a valid argument! :-)  We can discuss the implementation on IRC if
> you want.  It might be that we can slightly generalize ‘tuning-compiler’
> so that it works for go (perhaps there’s an option like ‘-march’ that we
> could use instead of setting ‘GOAMD’?).

I found -goarch, but it's for cross-compiling and wouldn't take
x86_64-v3 as an input.

The attached diff has 2 parts, the first wraps the go binary (and only
the go binary) with GOAMD or the like. The second part is commented out,
but is how I would've fixed the extra 'set-microarchitecture phase.

I'm pretty certain that I have the logic correct, but I'm not certain
that it's being applied. It probably needs (system* "export" "GOAMD"
#$psabi) or something similar, when I tried adjusting syncthing to
display (getenv "GOAMD") I was getting #f.

-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #1.2: wrap-go-binary.diff --]
[-- Type: text/plain, Size: 5183 bytes --]

diff --git a/guix/transformations.scm b/guix/transformations.scm
index 92d9c89c0e..0665f33178 100644
--- a/guix/transformations.scm
+++ b/guix/transformations.scm
@@ -441,6 +441,9 @@ (define tuning-compiler
       #~(begin
           (use-modules (ice-9 match))
 
+          (define psabi #$(gcc-architecture->micro-architecture-level
+                            micro-architecture))
+
           (define* (search-next command
                                 #:optional
                                 (path (string-split (getenv "PATH")
@@ -469,10 +472,25 @@ (define tuning-compiler
              (match (search-next (basename command))
                (#f (exit 127))
                (next
-                (apply execl next
+                 (if (and (search-next "go")
+                          (string=? next (search-next "go")))
+                   (cond
+                     ((string-prefix? "arm" psabi)
+                      (setenv "GOARM" (string-take-right psabi 1)))
+                     ((string-prefix? "powerpc" psabi)
+                      (setenv "GOPPC64" psabi))
+                     ((string-prefix? "x86_64" psabi)
+                      (setenv "GOAMD" (string-take-right psabi 2)))
+                     (else #t))
+                   '())
+                (apply
+                  execl next
                        (append (cons next arguments)
+                         (if (and (search-next "go")
+                                  (string=? next (search-next "go")))
+                           '()
                            (list (string-append "-march="
-                                                #$micro-architecture))))))))))
+                                                #$micro-architecture)))))))))))
 
     (define program
       (program-file (string-append "tuning-compiler-wrapper-" micro-architecture)
@@ -489,7 +507,8 @@ (define tuning-compiler
                          (for-each (lambda (program)
                                      (symlink #$program
                                               (string-append bin "/" program)))
-                                   '("cc" "gcc" "clang" "g++" "c++" "clang++")))))))
+                                   '("cc" "gcc" "clang" "g++" "c++" "clang++"
+                                     "go")))))))
 
 (define (build-system-with-tuning-compiler bs micro-architecture)
   "Return a variant of BS, a build system, that ensures that the compiler that
@@ -564,27 +583,31 @@ (define (build-system-with-tuning-compiler bs micro-architecture)
 
       (bag
         (inherit lowered)
-        (arguments
+        #;(arguments
           (substitute-keyword-arguments (bag-arguments lowered)
           ;; We add the tuning parameter after the default GO flags are set.
           ((#:phases phases '%standard-phases)
-             #~(modify-phases #$phases
-                 (add-after 'setup-go-environment 'set-microarchitecture
-                   (lambda _
-                     (cond
-                       ((string-prefix? "arm" #$psabi)
-                        (setenv "GOARM" (string-take-right #$psabi 1))
-                        (format #t "Setting GOARM to ~s."
-                                (getenv "GOARM")))
-                       ((string-prefix? "powerpc" #$psabi)
-                        (setenv "GOPPC64" #$psabi)
-                        (format #t "Setting GOPPC64 to ~s."
-                                (getenv "GOPPC64")))
-                       ((string-prefix? "x86_64" #$psabi)
-                        (setenv "GOAMD" (string-take-right #$psabi 2))
-                        (format #t "Setting GOAMD to ~s.\n"
-                                (getenv "GOAMD")))
-                       (else #t))))))))
+             ;; This phase is only in the go-build-system.
+             #~(if (assoc-ref #$phases 'setup-go-environment)
+                 (modify-phases #$phases
+                   (replace 'setup-go-environment
+                     (lambda* args
+                       (apply (assoc-ref #$phases 'setup-go-environment) args)
+                       (cond
+                         ((string-prefix? "arm" #$psabi)
+                          (setenv "GOARM" (string-take-right #$psabi 1))
+                          (format #t "Setting GOARM to ~s."
+                                  (getenv "GOARM")))
+                         ((string-prefix? "powerpc" #$psabi)
+                          (setenv "GOPPC64" #$psabi)
+                          (format #t "Setting GOPPC64 to ~s."
+                                  (getenv "GOPPC64")))
+                         ((string-prefix? "x86_64" #$psabi)
+                          (setenv "GOAMD" (string-take-right #$psabi 2))
+                          (format #t "Setting GOAMD to ~s.\n"
+                                  (getenv "GOAMD")))
+                         (else #t)))))
+                 #$phases))))
         (build-inputs
          ;; Arrange so that the compiler wrapper comes first in $PATH.
          `(("tuning-compiler" ,(tuning-compiler micro-architecture))

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#64188] [PATCH 0/8] More package tuning

[Josselin Poiret via Guix-patches via]

[-- Attachment #1: Type: text/plain, Size: 941 bytes --]

Hi Efraim,

Efraim Flashner <efraim@flashner.co.il> writes:

> I'm pretty certain that I have the logic correct, but I'm not certain
> that it's being applied. It probably needs (system* "export" "GOAMD"
> #$psabi) or something similar, when I tried adjusting syncthing to
> display (getenv "GOAMD") I was getting #f.

That system* call won't do anything: export is a built-in in most shells
that sets an environment variable for the current shell session.
system* doesn't run the command you're giving it through a shell, so it
won't find `export`, and if you use system instead (which would run it
through a shell) it will only take effect during the (system ...) call,
after which the shell session is torn down.  You really need to use the
guile setenv if you want it to have any effect on started programs (or
use the spawn interface to specify env variables for a specific
invocation).

Best,
-- 
Josselin Poiret

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 682 bytes --]

[bug#64188] [PATCH 0/8] More package tuning

[Ludovic Courtès]

Hi,

Efraim Flashner <efraim@flashner.co.il> skribis:

>> Not a valid argument! :-)  We can discuss the implementation on IRC if
>> you want.  It might be that we can slightly generalize ‘tuning-compiler’
>> so that it works for go (perhaps there’s an option like ‘-march’ that we
>> could use instead of setting ‘GOAMD’?).
>
> I found -goarch, but it's for cross-compiling and wouldn't take
> x86_64-v3 as an input.
>
> The attached diff has 2 parts, the first wraps the go binary (and only
> the go binary) with GOAMD or the like. The second part is commented out,
> but is how I would've fixed the extra 'set-microarchitecture phase.

At first sight it seems to me like it’s going in the right direction.

Perhaps we should create a separate ‘tuning-go-compiler’ for clarity,
and arrange to factorize ‘search-next’ between the two.

Let me know what you think!

Thanks,
Ludo’.

bug#64188: [PATCH 0/8] More package tuning

[Ludovic Courtès]

Hi Efraim,

Ludovic Courtès <ludovic.courtes@inria.fr> skribis:

> Efraim Flashner <efraim@flashner.co.il> skribis:
>
>>> Not a valid argument! :-)  We can discuss the implementation on IRC if
>>> you want.  It might be that we can slightly generalize ‘tuning-compiler’
>>> so that it works for go (perhaps there’s an option like ‘-march’ that we
>>> could use instead of setting ‘GOAMD’?).
>>
>> I found -goarch, but it's for cross-compiling and wouldn't take
>> x86_64-v3 as an input.
>>
>> The attached diff has 2 parts, the first wraps the go binary (and only
>> the go binary) with GOAMD or the like. The second part is commented out,
>> but is how I would've fixed the extra 'set-microarchitecture phase.
>
> At first sight it seems to me like it’s going in the right direction.
>
> Perhaps we should create a separate ‘tuning-go-compiler’ for clarity,
> and arrange to factorize ‘search-next’ between the two.
>
> Let me know what you think!

I see you pushed a variant as 1fd4f544b3065af225731462f3d3d647da781ee8,
neat.

Closing!

Ludo’.