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