Patch applied with a couple of helper patches. Some comments inline. On Sat, Nov 18, 2023 at 05:51:09PM +0100, Ekaitz Zarraga wrote: > Hi, > > Following Efraim's suggestion I arranged a new patch around the tuning > option. I needed to patch Zig itself to use baseline cpu by default, which > is safer in general for us. > > If anyone need a more specific cpu, can use the tuning option to choose it. > > Maybe I should split the changes in Zig and the tuning system, but if I do > they wont appear together, and they mostly only make sense if applied at > once. I split the patch and applied the one adjusting zig to use -Dcpu=baseline first. > Please let me know if you like this one better. > > Hope it's cool. > > Cheers, > Ekaitz > From 03616b9764278e05d74f8e8bf5d65b043603a437 Mon Sep 17 00:00:00 2001 > Message-ID: <03616b9764278e05d74f8e8bf5d65b043603a437.1700325640.git.ekaitz@elenq.tech> > From: Ekaitz Zarraga > Date: Sat, 18 Nov 2023 17:29:07 +0100 > Subject: [PATCH] gnu: zig: Make compiler tunable. > > In order to make Zig tunable, we have to make sure by default it does > the right thing, that is use the `baseline` CPU instead of the `native` > one because the tuning system only adds an extra flag to the compiler > command line execution. > > * gnu/packages/patches/zig-use-baseline-cpu-by-default.patch: Add file. > * gnu/packages/zig.scm(zig-0.10.1): Apply patch above. > * guix/transformations.scm(tuning-compiler): Add support for zig. Missing a change for gnu/local.mk > Change-Id: I40bd28071c97c0dd0a907c704072b52b26d2de28 > --- > .../zig-use-baseline-cpu-by-default.patch | 36 +++++++++++++++++++ > gnu/packages/zig.scm | 3 +- > guix/transformations.scm | 22 ++++++++---- > 3 files changed, 54 insertions(+), 7 deletions(-) > create mode 100644 gnu/packages/patches/zig-use-baseline-cpu-by-default.patch > > diff --git a/gnu/packages/patches/zig-use-baseline-cpu-by-default.patch b/gnu/packages/patches/zig-use-baseline-cpu-by-default.patch > new file mode 100644 > index 0000000000..be78d9c6c7 > --- /dev/null > +++ b/gnu/packages/patches/zig-use-baseline-cpu-by-default.patch > @@ -0,0 +1,36 @@ > +From 1dc188129950031243c5a0c80ec2562fab8ec549 Mon Sep 17 00:00:00 2001 > +From: Ekaitz Zarraga > +Date: Sat, 18 Nov 2023 15:04:16 +0100 > +Subject: [PATCH] Use `baseline` cpu by default. > + > +This helps Guix tune the package later. Tunning will only add > +`-Dcpu=whatever` which should override the standard behaviour. > + > +Zig by default uses `native`, which interferes with our build process. > +In our previous zig-build-system we chose to add `-Dcpu=baseline` flag > +in each `zig build` execution, but that doesn't allow us to tune the > +package later. Tunning is only designed to add extra flags in the > +command line call, and we already had one set for the baseline case. > +With this patch we set the standard behavior to `baseline` so we don't > +need to add the `-Dcpu=baseline` flag in the zig-build-system and we can > +tune with no issues. > +--- > + lib/std/zig/CrossTarget.zig | 2 +- > + 1 file changed, 1 insertion(+), 1 deletion(-) > + > +diff --git a/lib/std/zig/CrossTarget.zig b/lib/std/zig/CrossTarget.zig > +index 6c59a4a3a..f5ec065fe 100644 > +--- a/lib/std/zig/CrossTarget.zig > ++++ b/lib/std/zig/CrossTarget.zig > +@@ -12,7 +12,7 @@ const mem = std.mem; > + /// `null` means native. > + cpu_arch: ?Target.Cpu.Arch = null, > + > +-cpu_model: CpuModel = CpuModel.determined_by_cpu_arch, > ++cpu_model: CpuModel = CpuModel.baseline, > + > + /// Sparse set of CPU features to add to the set from `cpu_model`. > + cpu_features_add: Target.Cpu.Feature.Set = Target.Cpu.Feature.Set.empty, > +-- > +2.41.0 > + > diff --git a/gnu/packages/zig.scm b/gnu/packages/zig.scm > index dcca9a1121..82b83f44c2 100644 > --- a/gnu/packages/zig.scm > +++ b/gnu/packages/zig.scm > @@ -42,7 +42,8 @@ (define-public zig-0.10 > (file-name (git-file-name name version)) > (sha256 > (base32 "1sh5xjsksl52i4cfv1qj36sz5h0ln7cq4pdhgs3960mk8a90im7b")) > - (patches (search-patches "zig-do-not-link-against-librt.patch")))) > + (patches (search-patches "zig-do-not-link-against-librt.patch" > + "zig-use-Dcpu-baseline-by-default.patch")))) > (build-system cmake-build-system) > (inputs > (list clang-15 ; Clang propagates llvm. > diff --git a/guix/transformations.scm b/guix/transformations.scm > index 9cba6bedab..8964feb046 100644 > --- a/guix/transformations.scm > +++ b/guix/transformations.scm > @@ -439,7 +439,8 @@ (define tuning-compiler > actual compiler." > (define wrapper > #~(begin > - (use-modules (ice-9 match)) > + (use-modules (ice-9 match) > + (ice-9 string-fun)) > > (define psabi #$(gcc-architecture->micro-architecture-level > micro-architecture)) > @@ -486,11 +487,20 @@ (define tuning-compiler > (apply > execl next > (append (cons next arguments) > - (if (and (search-next "go") > + (cond > + ((and (search-next "go") > (string=? next (search-next "go"))) > - '() > - (list (string-append "-march=" > - #$micro-architecture))))))))))) > + '()) > + ((and (search-next "zig") > + (string=? next (search-next "zig")) Indentation is off, the and should end with string=? > + (list (string-append This I changed to `(,(string-append, otherwise it complains about receiving a list instead of a string. > + ;; https://issues.guix.gnu.org/67075#3 > + "-Dcpu=" > + (string-replace-substring > + #$micro-architecture "-" "_"))))) > + (else > + (list (string-append "-march=" > + #$micro-architecture)))))))))))) > > (define program > (program-file (string-append "tuning-compiler-wrapper-" micro-architecture) > @@ -508,7 +518,7 @@ (define tuning-compiler > (symlink #$program > (string-append bin "/" program))) > '("cc" "gcc" "clang" "g++" "c++" "clang++" > - "go"))))))) > + "go" "zig"))))))) > > (define (build-system-with-tuning-compiler bs micro-architecture) > "Return a variant of BS, a build system, that ensures that the compiler that > > base-commit: a514973413dc8179100ef8f0fcf41f5ac38c982f > -- > 2.41.0 > -- Efraim Flashner רנשלפ םירפא GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351 Confidentiality cannot be guaranteed on emails sent or received unencrypted