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

runtime: remove VZEROUPPER patch once Darwin <10.15.6 is not supported #41152

Open
cuonglm opened this issue Sep 1, 2020 · 9 comments
Open
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. early-in-cycle A change that should be done early in the 3 month dev cycle. FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin Performance
Milestone

Comments

@cuonglm
Copy link
Member

cuonglm commented Sep 1, 2020

Split out from #37174 (comment)

So once we stop supporting releases <10.15.6, we can get rid of the VZEROUPPER patch.

cc @randall77 @dmitshur

@cuonglm cuonglm changed the title runtime: remove VZEROUPPER patch once 10.15.6 is deprecated runtime: remove VZEROUPPER patch once Darwin <10.15.6 is not supported Sep 1, 2020
@randall77 randall77 added this to the Go1.17 milestone Sep 1, 2020
@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 1, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Sep 1, 2020

Apple has been making a new major macOS version each year, and Go has been dropping support for old macOS versions at the same rate. So we can estimate.

Go 1.11 started to require macOS 10.10 or later.
Go 1.13 started to require macOS 10.11 or later.
Go 1.15 started to require macOS 10.12 or later.
If that rate doesn't change, Go 1.23 will start to require macOS 10.16 (aka macOS 11.0) or later, and this optimization can be applied then.

@HenkPoley
Copy link

HenkPoley commented Nov 13, 2020

Potentially this issue was reintroduced (elsewhere?) in macOS 10.15.7 (19H15?): https://twitter.com/Cyan4973/status/1327023284974538754

@randall77
Copy link
Contributor

For future self, make sure the reproducers in #37174 perform ok before removing the VZEROUPPER.

@randall77
Copy link
Contributor

Just upgraded to 10.15.7 and it looks still fixed to me.
(The C/asm reproducer runs at the same speed regardless of whether the vzeroupper is commented out or not.)

@mdempsky
Copy link
Member

According to https://golang.org/doc/go1.16, Go 1.17 will still support macOS 10.13. So should this be deferred to Go 1.18 (or Go 1.23, per @dmitshur)?

@dmitshur
Copy link
Contributor

I understand this isn't so critical that we need to re-evaluate and bump every 6 months, so I'll put this in a future Go 1.23 milestone. If something changes before then, we can revisit this.

@dmitshur dmitshur modified the milestones: Go1.17, Go1.23 Apr 29, 2021
@dmitshur dmitshur added the early-in-cycle A change that should be done early in the 3 month dev cycle. label Apr 29, 2021
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
@gopherbot
Copy link

This issue is currently labeled as early-in-cycle for Go 1.23.
That time is now, so a friendly reminder to look at it again.

@gopherbot
Copy link

Change https://go.dev/cl/560955 mentions this issue: runtime: remove VZEROUPPER in asyncPreempt on darwin/amd64

@cuonglm
Copy link
Member Author

cuonglm commented Feb 20, 2024

I could not find a Mac Intel for testing https://go-review.googlesource.com/c/go/+/560955, any help on verifying the CL is welcome.

cc @golang/darwin

@dmitshur dmitshur added the FixPending Issues that have a fix which has not yet been reviewed or submitted. label Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. early-in-cycle A change that should be done early in the 3 month dev cycle. FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin Performance
Projects
Status: Triage Backlog
Development

No branches or pull requests

6 participants