[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#64188] [PATCH 0/8] More package tuning
From: |
Efraim Flashner |
Subject: |
[bug#64188] [PATCH 0/8] More package tuning |
Date: |
Tue, 18 Jul 2023 14:17:39 +0300 |
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
wrap-go-binary.diff
Description: Text document
signature.asc
Description: PGP signature