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: load after writebarrier call instead of spilling #15845

Closed
josharian opened this issue May 26, 2016 · 7 comments
Closed

cmd/compile: load after writebarrier call instead of spilling #15845

josharian opened this issue May 26, 2016 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone

Comments

@josharian
Copy link
Contributor

The inplace append optimization (CLs 21813 and 22197) use a trick to avoid a spill: When doing *dst = src via a write barrier, immediately do src = *dst. (Search for a comment like "load the value we just stored to avoid having to spill it" in ssa.go.)

If src is used again, it doesn't need to be spilled before the write barrier; it is a load either way, but we can skip the spill/store. If src isn't used again, the load will be eliminated as dead code.

Perhaps should we investigate extending this trick to all writebarrier-enabled pointer stores. Thoughts?

@josharian
Copy link
Contributor Author

@cherrymui you might be interested in this. :)

@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 10, 2016
@rsc
Copy link
Contributor

rsc commented Oct 20, 2016

/cc @RLH @aclements

@rsc rsc modified the milestones: Go1.9, Go1.8Maybe Oct 20, 2016
@aclements
Copy link
Member

@josharian, could you give an example of what the code would look like before and after with this change?

@josharian
Copy link
Contributor Author

josharian commented Oct 25, 2016

could you give an example of what the code would look like before and after

I'm not sure what kind of answer would be helpful here. The goal is to replace spill src, *dst = src, load src, use src with *dst = src, src = *dst, use src.

@aclements
Copy link
Member

That's actually exactly the kind of answer I was looking for. Thanks.

Is the goal here just to reduce the needed spill space? It's not at all obvious to me that the new memory access pattern would be better.

This obviously has weird effects if the program has races, but maybe we're okay with that.

Alternatively, since we're passing src to the write barrier, should we just have the write barrier return src? (Or if the write barrier promises not to write to its argument, it can be reloaded straight from the argument slot, if that's not too scary. :)

Even more alternatively, @RLH and I have talked a bit about using an SSB-style write barrier, mostly to improve register utilization around write barriers. This would have a very short fast path that just added the pointer to a per-P buffer (with a slow path to handle when this buffer overflows). The fast path could be coded in assembly to guarantee that it only uses some small set of registers, so the compiler could keep the rest across the write barrier call. The slow path would probably just save all registers; the type information doesn't matter here because it wouldn't be preemptible anyway.

@josharian
Copy link
Contributor Author

Alternatively, since we're passing src to the write barrier, should we just have the write barrier return src?

This sounds good to me--at least worth trying out.

The fast path could be coded in assembly to guarantee that it only uses some small set of registers, so the compiler could keep the rest across the write barrier call.

This also sounds promising.

@josharian josharian modified the milestones: Go1.10, Go1.9 May 18, 2017
@bradfitz bradfitz modified the milestones: Go1.10, Go1.11 Nov 28, 2017
@josharian
Copy link
Contributor Author

The fast path could be coded in assembly to guarantee that it only uses some small set of registers, so the compiler could keep the rest across the write barrier call.

This happened, and it's great.

@golang golang locked and limited conversation to collaborators Mar 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
None yet
Development

No branches or pull requests

6 participants