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

cmd/compile: unneeded stack checks and stack handling #32197

Closed
mariecurried opened this issue May 23, 2019 · 4 comments
Closed

cmd/compile: unneeded stack checks and stack handling #32197

mariecurried opened this issue May 23, 2019 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone
Backlog

Comments

@mariecurried
Copy link

What version of Go are you using (go version)?

$ go version
go version devel +24b4301 Wed May 22 02:10:36 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What did you do?

I compiled the following program https://play.golang.org/p/STj160cWl4l, simplified from a bigger code base.

What did you expect to see?

I expected function a1 to be compiled to a simple return, without the need for the stack checks and stack handling after a2 returns, given that a2 is compiled to a simple return.
My expectation came from the fact that in this other program (https://play.golang.org/p/VBBQJvevHxw), in which a2 still compiles to the same code, a1 doesn't have the stack checks/stack handling.

What did you see instead?

Instead, a1 was compiled with stack checks/stack handling.
@randall77
Copy link
Contributor

The reason for this discrepancy is that in your second example, a2 is inlined into a1. a1 is then a leaf with a small stack frame and requires no stack check. In your first example, a2 is not inlined because at inlining time, the body of a2 looked too big to inline. So a1 is no longer a leaf and requires at least some SP manipulation.

I'm surprised it has a stack check though. I thought we elided the stack checks if the call subtree all fit in the guard area. That may be the bug here.

@randall77 randall77 added this to the Go1.14 milestone May 23, 2019
@cherrymui
Copy link
Member

Currently, the stack bound check elimination is not interprocedural. We assemble one function at a time, and we don't look at the call graph. Currently we only eliminate stack bound check for leaf-ish functions (allowing calls to some special functions like duffzero or some panic functions).

@cherrymui
Copy link
Member

cherrymui commented May 24, 2019 •
edited

The linker computes the call graph, and check that the stack usage of nosplit functions doesn't go beyond the stack guard. We could imagine something in the linker that after the call graph computation, if it sees a subtree doesn't use much stack it can change the call target to skip over the stack bound check prologue (and/or change the prologue to NOPs).

@julieqiu julieqiu added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 28, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@mariecurried
Copy link
Author

No longer reproduces (fixed in 1.18).

@golang golang locked and limited conversation to collaborators Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
6 participants
@rsc @julieqiu @randall77 @gopherbot @cherrymui @mariecurried