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

Jump to bottom

runtime: procyield doesn't use yield instruction on ARM, unlike ARM64 #16663

Closed
swolchok opened this issue Aug 10, 2016 · 6 comments
Closed

runtime: procyield doesn't use yield instruction on ARM, unlike ARM64 #16663

swolchok opened this issue Aug 10, 2016 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.9

Comments

@swolchok
Copy link

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
    Master
  2. What operating system and processor architecture are you using (go env)?
    ARM
  3. What did you do?
    If possible, provide a recipe for reproducing the error.
    A complete runnable program is good.
    A link on play.golang.org is best.

code inspection:

go/src/runtime/asm_arm.s

Line 971 in 690de51

yieldloop:

I have not had a problem with this myself, just happened to notice it.

  1. What did you expect to see?

Use of the YIELD instruction, which exists on ARMv6K and newer per http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0204j/Cjafcggi.html Presumably, this would need some kind of architecture gating, because Go supports ARMv5 and ARMv6: https://github.com/golang/go/wiki/GoArm

  1. What did you see instead?

Busy-wait with no YIELD, unlike the ARM64 version:

go/src/runtime/asm_arm64.s

Line 473 in 690de51

YIELD

Note that it could be the case that the ARM64 version is in error, since runtime.procyield takes a loop count and (according to the linked ARM documentation) YIELD may cause the thread to be suspended.
@bradfitz bradfitz changed the title runtime.procyield doesn't use yield instruction on ARM, unlike ARM64 runtime: procyield doesn't use yield instruction on ARM, unlike ARM64 Aug 10, 2016
@bradfitz
Copy link
Contributor

/cc @cherrymui

@quentinmit quentinmit added this to the Go1.8 milestone Aug 26, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 11, 2016
@cherrymui
Copy link
Member

cherrymui commented Nov 1, 2016 •
edited

add @minux, who might know the original idea.
Architecture gating might be a reason. There are not so much code in the runtime that is architecture gated.

@minux
Copy link
Member

minux commented Nov 1, 2016 via email

@minux minux mentioned this issue Nov 7, 2016
Closed
@aclements
Copy link
Member

@minux, based on my reading of the ARM spec, it sounds like CPUs that don't support YIELD will just interpret it as a NOP. If so, it would be harmless to add to procyield.

@rsc rsc modified the milestones: Go1.9, Go1.8 Nov 11, 2016
@aclements
Copy link
Member

Taking another look at this, ARMv7 skirts around clearly stating that YIELD is a NOP on earlier architectures, but I'm pretty sure it is.

Based on the instruction encoding, on ARMv5 it will be interpreted as an MSR instruction with

  R=0
  field_mask=0000
  SBO=1111
  rotate_imm=0000
  8_bit_immediate=00000001

If I'm following the pseudo-code correctly, the ultimate effect of this will be CPSR = CPSR. In other words, a NOP.

@gopherbot
Copy link

CL https://golang.org/cl/45250 mentions this issue.

@golang golang locked and limited conversation to collaborators Jun 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.9
Development

No branches or pull requests
8 participants
@bradfitz @swolchok @rsc @quentinmit @minux @aclements @gopherbot @cherrymui