diff mbox series

[bug#67075] build: zig-build-system: Add CPU option

Message ID WqKhVsnCWxUfZxvdoFcRDtlW88FnHPZ5QqzSVDUYUOipndmdsfAe3XLPXTPookk6OgqJ8bFaOVCOVAlGWOR3-hs4TD-EtBWBlmm_tAvS75s=@elenq.tech
State New
Headers show
Series [bug#67075] build: zig-build-system: Add CPU option | expand

Commit Message

Ekaitz Zarraga Nov. 11, 2023, 1:09 p.m. UTC
From a647a8ee689022cafef4bab05784b32b1c97bee7 Mon Sep 17 00:00:00 2001
Message-ID: <a647a8ee689022cafef4bab05784b32b1c97bee7.1699708101.git.ekaitz@elenq.tech>
From: Ekaitz Zarraga <ekaitz@elenq.tech>
Date: Sat, 11 Nov 2023 14:05:23 +0100
Subject: [PATCH] build: zig-build-system: Add CPU option

Zig packages are optimized by default, adding `-Dcpu=baseline` to the
build command builds them for an standard cpu that should work in every
machine.

This change sets that by default but also allows users to choose their
cpu by the `#:zig-cpu` argument.

* guix/build-system/zig.scm (build): add zig-cpu
* guix/build/zig-build-system.scm (zig-build) add zig-cpu

Change-Id: Ib4b2124179e7b5492e7c77c64e1f8336832032ea
---
 guix/build-system/zig.scm       | 2 ++
 guix/build/zig-build-system.scm | 2 ++
 2 files changed, 4 insertions(+)


base-commit: af6105afc67a15a491a0a4fd18a28c9f801a0b94

Comments

Efraim Flashner Nov. 12, 2023, 2:38 p.m. UTC | #1
What are the values that the compiler can take for this flag?  Also,
this seems like something that can be addressed with the tuning
mechanism, so we can run 'guix build foo --tune' and it'll do The Right
Thing™.

Alternatively, if we do go this route, you still need to update the
documentation.

On Sat, Nov 11, 2023 at 01:09:07PM +0000, Ekaitz Zarraga wrote:
> From a647a8ee689022cafef4bab05784b32b1c97bee7 Mon Sep 17 00:00:00 2001
> Message-ID: <a647a8ee689022cafef4bab05784b32b1c97bee7.1699708101.git.ekaitz@elenq.tech>
> From: Ekaitz Zarraga <ekaitz@elenq.tech>
> Date: Sat, 11 Nov 2023 14:05:23 +0100
> Subject: [PATCH] build: zig-build-system: Add CPU option
> 
> Zig packages are optimized by default, adding `-Dcpu=baseline` to the
> build command builds them for an standard cpu that should work in every
> machine.
> 
> This change sets that by default but also allows users to choose their
> cpu by the `#:zig-cpu` argument.
> 
> * guix/build-system/zig.scm (build): add zig-cpu
> * guix/build/zig-build-system.scm (zig-build) add zig-cpu
> 
> Change-Id: Ib4b2124179e7b5492e7c77c64e1f8336832032ea
> ---
>  guix/build-system/zig.scm       | 2 ++
>  guix/build/zig-build-system.scm | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/guix/build-system/zig.scm b/guix/build-system/zig.scm
> index 16b8a712cc..f90e76104e 100644
> --- a/guix/build-system/zig.scm
> +++ b/guix/build-system/zig.scm
> @@ -47,6 +47,7 @@ (define* (zig-build name inputs
>                      source
>                      (tests? #t)
>                      (test-target #f)
> +                    (zig-cpu #f)
>                      (zig-build-flags ''())
>                      (zig-test-flags ''())
>                      (zig-release-type #f)
> @@ -67,6 +68,7 @@ (define* (zig-build name inputs
>                       #:source #+source
>                       #:system #$system
>                       #:test-target #$test-target
> +                     #:zig-cpu #$zig-cpu
>                       #:zig-build-flags #$zig-build-flags
>                       #:zig-test-flags #$zig-test-flags
>                       #:zig-release-type #$zig-release-type
> diff --git a/guix/build/zig-build-system.scm b/guix/build/zig-build-system.scm
> index d414ebfb17..99a81314d4 100644
> --- a/guix/build/zig-build-system.scm
> +++ b/guix/build/zig-build-system.scm
> @@ -44,6 +44,7 @@ (define* (set-zig-global-cache-dir #:rest args)
>    (setenv "ZIG_GLOBAL_CACHE_DIR" global-cache-dir))
>  
>  (define* (build #:key
> +                zig-cpu
>                  zig-build-flags
>                  zig-release-type       ;; "safe", "fast" or "small" empty for a
>                                         ;; debug build"
> @@ -59,6 +60,7 @@ (define* (build #:key
>                       ,@(if zig-release-type
>                           (list (string-append "-Drelease-" zig-release-type))
>                           '())
> +                     ,(string-append "-Dcpu=" (or zig-cpu "baseline"))
>                       ,@zig-build-flags)))
>    (format #t "running: ~s~%" call)
>    (apply invoke call)))
> 
> base-commit: af6105afc67a15a491a0a4fd18a28c9f801a0b94
> -- 
> 2.41.0
> 
> 
> 
> 
>
Ekaitz Zarraga Nov. 12, 2023, 4:38 p.m. UTC | #2
Hi Efraim,

On Sunday, November 12th, 2023 at 15:38, Efraim Flashner <efraim@flashner.co.il> wrote:

> What are the values that the compiler can take for this flag? Also,
> this seems like something that can be addressed with the tuning
> mechanism, so we can run 'guix build foo --tune' and it'll do The Right
> Thing™.

It's not that simple. I talked with Andrew Kelley and he told me the values
are obtained from LLVM and they have some patching. I checked the list and 
they are similar to those we use in the -march field:
https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html#x86-Options

The full list is accessible in `zig targets` and it's very extensive.

I don't know if we can just rely on that mechanism...

> Alternatively, if we do go this route, you still need to update the
> documentation.

That's very true. I will.

> On Sat, Nov 11, 2023 at 01:09:07PM +0000, Ekaitz Zarraga wrote:
> 
> > From a647a8ee689022cafef4bab05784b32b1c97bee7 Mon Sep 17 00:00:00 2001
> > Message-ID: a647a8ee689022cafef4bab05784b32b1c97bee7.1699708101.git.ekaitz@elenq.tech
> > From: Ekaitz Zarraga ekaitz@elenq.tech
> > Date: Sat, 11 Nov 2023 14:05:23 +0100
> > Subject: [PATCH] build: zig-build-system: Add CPU option
> > 
> > Zig packages are optimized by default, adding `-Dcpu=baseline` to the
> > build command builds them for an standard cpu that should work in every
> > machine.
> > 
> > This change sets that by default but also allows users to choose their
> > cpu by the `#:zig-cpu` argument.
> > 
> > * guix/build-system/zig.scm (build): add zig-cpu
> > * guix/build/zig-build-system.scm (zig-build) add zig-cpu
> > 
> > Change-Id: Ib4b2124179e7b5492e7c77c64e1f8336832032ea
> > ---
> > guix/build-system/zig.scm | 2 ++
> > guix/build/zig-build-system.scm | 2 ++
> > 2 files changed, 4 insertions(+)
> > 
> > diff --git a/guix/build-system/zig.scm b/guix/build-system/zig.scm
> > index 16b8a712cc..f90e76104e 100644
> > --- a/guix/build-system/zig.scm
> > +++ b/guix/build-system/zig.scm
> > @@ -47,6 +47,7 @@ (define* (zig-build name inputs
> > source
> > (tests? #t)
> > (test-target #f)
> > + (zig-cpu #f)
> > (zig-build-flags ''())
> > (zig-test-flags ''())
> > (zig-release-type #f)
> > @@ -67,6 +68,7 @@ (define* (zig-build name inputs
> > #:source #+source
> > #:system #$system
> > #:test-target #$test-target
> > + #:zig-cpu #$zig-cpu
> > #:zig-build-flags #$zig-build-flags
> > #:zig-test-flags #$zig-test-flags
> > #:zig-release-type #$zig-release-type
> > diff --git a/guix/build/zig-build-system.scm b/guix/build/zig-build-system.scm
> > index d414ebfb17..99a81314d4 100644
> > --- a/guix/build/zig-build-system.scm
> > +++ b/guix/build/zig-build-system.scm
> > @@ -44,6 +44,7 @@ (define* (set-zig-global-cache-dir #:rest args)
> > (setenv "ZIG_GLOBAL_CACHE_DIR" global-cache-dir))
> > 
> > (define* (build #:key
> > + zig-cpu
> > zig-build-flags
> > zig-release-type ;; "safe", "fast" or "small" empty for a
> > ;; debug build"
> > @@ -59,6 +60,7 @@ (define* (build #:key
> > ,@(if zig-release-type
> > (list (string-append "-Drelease-" zig-release-type))
> > '())
> > + ,(string-append "-Dcpu=" (or zig-cpu "baseline"))
> > ,@zig-build-flags)))
> > (format #t "running: ~s~%" call)
> > (apply invoke call)))
> > 
> > base-commit: af6105afc67a15a491a0a4fd18a28c9f801a0b94
> > --
> > 2.41.0
> 
> 
> --
> 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
Ekaitz Zarraga Nov. 12, 2023, 4:41 p.m. UTC | #3
Extra information I missed in the previous email:

> andrewrk: there is close to a 1-1 correspondence. zig uses underscores instead of dashes for naming these

So we can use a translation function from the tuning capabilites we already have and see how it goes.

Cheers
diff mbox series

Patch

diff --git a/guix/build-system/zig.scm b/guix/build-system/zig.scm
index 16b8a712cc..f90e76104e 100644
--- a/guix/build-system/zig.scm
+++ b/guix/build-system/zig.scm
@@ -47,6 +47,7 @@  (define* (zig-build name inputs
                     source
                     (tests? #t)
                     (test-target #f)
+                    (zig-cpu #f)
                     (zig-build-flags ''())
                     (zig-test-flags ''())
                     (zig-release-type #f)
@@ -67,6 +68,7 @@  (define* (zig-build name inputs
                      #:source #+source
                      #:system #$system
                      #:test-target #$test-target
+                     #:zig-cpu #$zig-cpu
                      #:zig-build-flags #$zig-build-flags
                      #:zig-test-flags #$zig-test-flags
                      #:zig-release-type #$zig-release-type
diff --git a/guix/build/zig-build-system.scm b/guix/build/zig-build-system.scm
index d414ebfb17..99a81314d4 100644
--- a/guix/build/zig-build-system.scm
+++ b/guix/build/zig-build-system.scm
@@ -44,6 +44,7 @@  (define* (set-zig-global-cache-dir #:rest args)
   (setenv "ZIG_GLOBAL_CACHE_DIR" global-cache-dir))
 
 (define* (build #:key
+                zig-cpu
                 zig-build-flags
                 zig-release-type       ;; "safe", "fast" or "small" empty for a
                                        ;; debug build"
@@ -59,6 +60,7 @@  (define* (build #:key
                      ,@(if zig-release-type
                          (list (string-append "-Drelease-" zig-release-type))
                          '())
+                     ,(string-append "-Dcpu=" (or zig-cpu "baseline"))
                      ,@zig-build-flags)))
   (format #t "running: ~s~%" call)
   (apply invoke call)))