-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
Comments
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. |
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? |
Neither the runtime nor compiler use AVX512 and I dont expect them to do so anytime soon. |
@golang/release This is in the 1.18 milestone; time to move to 1.19? Thanks. |
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. |
Change https://go.dev/cl/386116 mentions this issue: |
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>
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. |
Change https://go.dev/cl/394614 mentions this issue: |
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
The text was updated successfully, but these errors were encountered: