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: procyield doesn't use yield instruction on ARM, unlike ARM64 #16663

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

Comments

Projects
None yet
8 participants
@swolchok
Copy link

commented Aug 10, 2016

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:

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:

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

This comment has been minimized.

Copy link
Member

commented Aug 10, 2016

@quentinmit quentinmit added this to the Go1.8 milestone Aug 26, 2016

@quentinmit quentinmit added the NeedsFix label Oct 11, 2016

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2016

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

This comment has been minimized.

Copy link
Member

commented Nov 1, 2016

@aclements

This comment has been minimized.

Copy link
Member

commented Nov 9, 2016

@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

This comment has been minimized.

Copy link
Member

commented Jun 9, 2017

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

This comment has been minimized.

Copy link

commented Jun 9, 2017

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

@gopherbot gopherbot closed this in bdc6418 Jun 9, 2017

@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.
You can’t perform that action at this time.