Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/build: Add GOAMD64 builders for go1.18 #48505

Closed
martisch opened this issue Sep 21, 2021 · 8 comments
Closed

x/build: Add GOAMD64 builders for go1.18 #48505

martisch opened this issue Sep 21, 2021 · 8 comments
Labels
Builders NeedsFix new-builder
Milestone

Comments

@martisch
Copy link
Contributor

@martisch martisch commented Sep 21, 2021

WIth #45453 we are introducing new compilation variants for AMD64. We should have dedicated builders that test these as these will make the go compiler use new instructions and generate different binaries in go1.18.

As far as I understand none of the current try or slow bots will test binaries that are generated with the new v2, v3, v4 setting of GOAMD64.

We can likely leave the v4 builder out for now as its unlikely to be used soon and if so we can defer having one until then.
If we need one we need to make sure that one runs on a cloud machine hardware that supports AVX512 (e.g. skylake server): https://en.wikipedia.org/wiki/X86-64#Microarchitecture_levels

/cc @mdempsky @dmitshur @cagedmantis @randall77

@gopherbot gopherbot added the Builders label Sep 21, 2021
@gopherbot gopherbot added this to the Unreleased milestone Sep 21, 2021
@martisch martisch removed this from the Unreleased milestone Sep 21, 2021
@martisch martisch added this to the Go1.18 milestone Sep 21, 2021
@dr2chase dr2chase added the NeedsFix label Sep 24, 2021
@mvdan
Copy link
Member

@mvdan mvdan commented Oct 19, 2021

The builder would already have caught one regression that broke make.bash with v3 :) #49061

Since GOARCH=amd64 is very common, and the majority of machines out there can build with GOAMD64=v3, I would hope that such a builder would be a trybot, not a slowbot. This should help catch regressions before merging, too.

@dmitshur dmitshur added this to Planned in Go Release Team Oct 26, 2021
@zx2c4
Copy link
Contributor

@zx2c4 zx2c4 commented Nov 4, 2021

We can likely leave the v4 builder out for now as its unlikely to be used soon

Worth noting that laptops are now shipping with AVX512, with this year's Tiger Lake laptops being somewhat popular. Or did you mean that Go doesn't yet use any avx512 instructions so it wouldn't be in use anyway?

@martisch
Copy link
Contributor Author

@martisch martisch commented Nov 4, 2021

Neither the runtime nor compiler use AVX512 and I dont expect them to do so anytime soon.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 29, 2022

@golang/release This is in the 1.18 milestone; time to move to 1.19? Thanks.

@dmitshur
Copy link
Contributor

@dmitshur dmitshur commented Feb 16, 2022

This issue remains in our queue, but moving out of the 1.18 milestone since it can happen independently of the release, and is not deemed critical enough to block it.

Since I've already paged in the relevant context, sent CL 386116 for review to get the ball rolling on this. Once it works well, we can consider making it a TryBot.

@randall77 made a suggestion of starting with just the v3 builder first, leaving v2 (and v4) for later, which is what I've done so far.

@dmitshur dmitshur removed this from the Go1.18 milestone Feb 16, 2022
@dmitshur dmitshur added this to the Unreleased milestone Feb 16, 2022
@gopherbot
Copy link

@gopherbot gopherbot commented Feb 16, 2022

Change https://go.dev/cl/386116 mentions this issue: dashboard: add linux-amd64-goamd64v3 builder

gopherbot pushed a commit to golang/build that referenced this issue Feb 18, 2022
The purpose of this builder will be to test Go with a non-default
value of the new GOAMD64 environment variable.

For golang/go#48505.
Updates golang/go#45453.

Change-Id: Ic7bf0bd45f3539530ac6540cc3609f87cfdab6f7
Reviewed-on: https://go-review.googlesource.com/c/build/+/386116
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alex Rakoczy <alex@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
@dmitshur dmitshur moved this from Planned to In Progress in Go Release Team Mar 2, 2022
@dmitshur
Copy link
Contributor

@dmitshur dmitshur commented Mar 22, 2022

Data so far shows the builder is working well. It caught one small test problem in CL 391694 (fixed in CL 392994).

It's hard to definitively predict whether making it a TryBot by default is worthwhile, especially since it can be requested as a SlowBot when needed. I'll mail a CL that just removes the known issue for now, and we can come back to the decision later, when there's more data.

@gopherbot
Copy link

@gopherbot gopherbot commented Mar 22, 2022

Change https://go.dev/cl/394614 mentions this issue: dashboard: remove linux-amd64-goamd64v3 known issue

Go Release Team automation moved this from In Progress to Done Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Builders NeedsFix new-builder
Projects
Development

No branches or pull requests

8 participants