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

cmd/compile: enable Duff's device on darwin/arm64 #54189

Closed
erifan opened this issue Aug 2, 2022 · 3 comments
Closed

cmd/compile: enable Duff's device on darwin/arm64 #54189

erifan opened this issue Aug 2, 2022 · 3 comments
Assignees
Labels
arch-arm64 compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin Performance
Milestone

Comments

@erifan
Copy link

erifan commented Aug 2, 2022

This is disabled because Darwin linker does not support BR26 relocation with non-zero addend, see #16724. I wonder if we can find a way to work around this issue, since the performance impact of Duff's device is pretty obvious.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Aug 2, 2022
@dmitshur dmitshur added this to the Backlog milestone Aug 2, 2022
@dmitshur dmitshur added Performance NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 2, 2022
@dmitshur
Copy link
Contributor

dmitshur commented Aug 2, 2022

CC @golang/compiler.

@cherrymui
Copy link
Member

There have been several changes to the Go linker. I'm not sure the BR26 issue is still relevant. We can try to enable it.

If it still matters, we can use the label symbol mechanism, which is what we do on Windows ARM64.

I can give it a try.

@gopherbot
Copy link

Change https://go.dev/cl/420894 mentions this issue: cmd/compile, cmd/link: enable Duff's device on darwin/arm64

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 3, 2022
@dmitshur dmitshur modified the milestones: Backlog, Go1.20 Aug 3, 2022
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
Duff's device was disabled on darwin/arm64 because the darwin
linker couldn't handle a branch relocation with non-zero addend.
This is no longer the case now. The darwin linker can handle it
just fine. So enable it.

Fixes golang#54189.

Change-Id: Ida7ebafe6eb01db1af5bb8ae60a62491da5eabdf
Reviewed-on: https://go-review.googlesource.com/c/go/+/420894
Reviewed-by: Eric Fang <eric.fang@arm.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Aug 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-arm64 compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin Performance
Projects
None yet
Development

No branches or pull requests

5 participants