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.

Sign up for GitHub

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

cmd/compile: document go:systemstack comment #12454

Closed
ianlancetaylor opened this issue Sep 2, 2015 · 5 comments
Closed

cmd/compile: document go:systemstack comment #12454

ianlancetaylor opened this issue Sep 2, 2015 · 5 comments
Labels
FrozenDueToAge
Milestone
Go1.6

Comments

@ianlancetaylor
Copy link
Contributor

Now that the runtime has been entirely converted from C to Go, I believe we can remove the go:systemstack comment. It follows that we can remove the Cfunc field from Lsym, remove all the uses of it, and remove the morestackc function.

I see exactly one remaining use of go:systemstack, on persistentalloc1 in runtime/malloc.go.

CC @randall77 in case you disagree.
@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Sep 2, 2015
@randall77
Copy link
Contributor

I don't think we should get rid of this feature. It is nice to annotate and check that routines that should not run on a normal Go stack don't in fact do that. We don't have any C code that needs this checking anymore, but there is Go code (and assembly, probably) that does. We should probably be using go:systemstack in other places, for example in all the stack copying code, signal handlers, etc.

Maybe a rename of Cfunc and morestackc is in order, though.

@ianlancetaylor ianlancetaylor changed the title cmd/compile: remove go:systemstack comment cmd/compile: document go:systemstack comment Sep 2, 2015
@ianlancetaylor
Copy link
Contributor Author

SGTM. I've changed this issue to call for documenting it, presumably in cmd/compile/doc.go.

@minux
Copy link
Member

minux commented Sep 2, 2015 via email

@ianlancetaylor
Copy link
Contributor Author

That works for me too.

@minux minux mentioned this issue Sep 3, 2015
Closed
@gopherbot
Copy link

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

@minux minux closed this as completed in dac87e9 Sep 4, 2015
@golang golang locked and limited conversation to collaborators Sep 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.6
Development

No branches or pull requests
4 participants
@minux @ianlancetaylor @randall77 @gopherbot