Navigation Menu

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: unnecessary write barriers to stack #17330

Closed
aclements opened this issue Oct 3, 2016 · 4 comments
Closed

cmd/compile: unnecessary write barriers to stack #17330

aclements opened this issue Oct 3, 2016 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.8

Comments

@aclements
Copy link
Member

There's some situation in which the compiler generates unnecessary write barriers when writing to a pointer in the current stack frame. Here's a simple script to find these (it may have false positives, but I didn't find any when flipping through them):

go tool objdump `which go` | awk '/LEAQ .*\(SP\),/ {lea=$0;next} /writebarrierptr/ && lea {if (text){print text;text=""};print lea;print} $4!="MOVQ"{lea=""} /^TEXT/ {text=$0}'

These are currently harmless. However, with the hybrid barrier it's important not to generate these because the stack slot being written to may be dead prior to this write and currently contain junk (such as a pointer to a dead object).

I flipped through several of these but there wasn't an obvious pattern to me.

/cc @dr2chase @cherrymui @randall77
@cherrymui
Copy link
Member

In gc/ssa.go:

        if rhs != nil && rhs.Op == OAPPEND {
            // The frontend gets rid of the write barrier to enable the special OAPPEND
            // handling above, but since this is not a special case, we need it.
            // TODO: just add a ptr graying to the end of growslice?
            // TODO: check whether we need to provide special handling and a write barrier
            // for ODOTTYPE and ORECV also.
            // They get similar wb-removal treatment in walk.go:OAS.
            needwb = true
        }

@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Oct 3, 2016
@cherrymui
cmd/compile: remove unnecessary write barriers for APPEND
d0e92f6 
Updates #17330.

Change-Id: I83fe80139a2213f3169db884b84a4c3bd15b58b6
Reviewed-on: https://go-review.googlesource.com/30140
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
@quentinmit quentinmit added this to the Go1.8 milestone Oct 3, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 3, 2016
@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Oct 10, 2016
@cherrymui
cmd/compile: remove some write barriers for stack writes
6833175 
This, along with CL 30140, removes ~50% of stack write barriers
mentioned in issue #17330. The remaining are most due to Phi and
FwdRef, which is not resolved when building SSA. We might be
able to do it at a later stage where Phi and Copy propagations
are done, but matching an if-(store-store-call)+ sequence seems
not very pleasant.

Updates #17330.

Change-Id: Iaa36c7b1f4c4fc3dc10a27018a3b0e261094cb21
Reviewed-on: https://go-review.googlesource.com/30290
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Oct 25, 2017
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.8
Development

No branches or pull requests
4 participants
@quentinmit @aclements @gopherbot @cherrymui