Skip to content

cmd/compile: insert write barriers in SSA #17583

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
Closed
aclements opened this issue Oct 25, 2016 · 10 comments
Closed

cmd/compile: insert write barriers in SSA #17583

aclements opened this issue Oct 25, 2016 · 10 comments
Labels
FrozenDueToAge
Milestone
Go1.9

Comments

@aclements
Copy link
Member

Currently we insert write barriers in the front end. Once https://golang.org/cl/31131 is in, we'll insert them in the front end and then remove some of them in the back end. This seems rather circuitous, but is necessary in the current structure because the front end has only partial information and has to guess at what the back end will do and be conservative.

We should switch to doing all write barrier insertion in the back end. That is, pass things through as regular assignments and stores until late in SSA (before lowering), and only then determine whether the write needs write barriers. This should have several advantages:

  1. We can be less conservative, since we have more information. For example, the new removal pass detects write barriers to the local stack frame. The front end tries to figure this out, but isn't always successful. The back end knows.
  2. We can do more sophisticated analysis. For example, basic flow analysis would make it possible to eliminate many write barriers with the hybrid barrier, but is too hard to do correctly in the front end.
  3. I suspect several of the funny write barrier cases in the front end around things like append would fall out naturally.
  4. It just makes sense.

/cc @cherrymui @randall77
@aclements aclements added this to the Go1.9Early milestone Oct 25, 2016
@cherrymui cherrymui mentioned this issue Feb 10, 2017
Closed
@gopherbot
Copy link
Contributor

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

@gopherbot
Copy link
Contributor

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

@gopherbot
Copy link
Contributor

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

@gopherbot
Copy link
Contributor

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

@gopherbot
Copy link
Contributor

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

gopherbot pushed a commit that referenced this issue Feb 17, 2017
@cherrymui
cmd/compile: redo writebarrier pass
c4ef597 
SSA's writebarrier pass requires WB store ops are always at the
end of a block. If we move write barrier insertion into SSA and
emits normal Store ops when building SSA, this requirement becomes
impractical -- it will create too many blocks for all the Store
ops.

Redo SSA's writebarrier pass, explicitly order values in store
order, so it no longer needs this requirement.

Updates #17583.
Fixes #19067.

Change-Id: I66e817e526affb7e13517d4245905300a90b7170
Reviewed-on: https://go-review.googlesource.com/36834
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: David Chase <[email protected]>
gopherbot pushed a commit that referenced this issue Mar 3, 2017
@cherrymui
cmd/compile: get rid of "volatile" in SSA
5bfd1ef 
A value is "volatile" if it is a pointer to the argument region
on stack which will be clobbered by function call. This is used
to make sure the value is safe when inserting write barrier calls.
The writebarrier pass can tell whether a value is such a pointer.
Therefore no need to mark it when building SSA and thread this
information through.

Passes "toolstash -cmp" on std.

Updates #17583.

Change-Id: Idc5fc0d710152b94b3c504ce8db55ea9ff5b5195
Reviewed-on: https://go-review.googlesource.com/36835
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
gopherbot pushed a commit that referenced this issue Mar 16, 2017
@cherrymui
cmd/compile: pass types on SSA Store/Move/Zero ops
211c8c9 
For SSA Store/Move/Zero ops, attach the type of the value being
stored to the op as the Aux field. This type will be used for
write barrier insertion (in a followup CL). Since SSA passes
do not accurately propagate types of values (because of type
casting), we can't simply use type of the store's arguments
for write barrier insertion.

Passes "toolstash -cmp" on std.

Updates #17583.

Change-Id: I051d5e5c482931640d1d7d879b2a6bb91f2e0056
Reviewed-on: https://go-review.googlesource.com/36838
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
gopherbot pushed a commit that referenced this issue Mar 16, 2017
@cherrymui
cmd/compile: move write barrier insertion to SSA
9ebf3d5 
When the compiler insert write barriers, the frontend makes
conservative decisions at an early stage. This sometimes have
false positives because of the lack of information, for example,
writes on stack. SSA's writebarrier pass identifies writes on
stack and eliminates write barriers for them.

This CL moves write barrier insertion into SSA. The frontend no
longer makes decisions about write barriers, and simply does
normal assignments and emits normal Store ops when building SSA.
SSA writebarrier pass inserts write barrier for Stores when needed.
There, it has better information about the store because Phi and
Copy propagation are done at that time.

This CL only changes StoreWB to Store in gc/ssa.go. A followup CL
simplifies SSA building code.

Updates #17583.

Change-Id: I4592d9bc0067503befc169c50b4e6f4765673bec
Reviewed-on: https://go-review.googlesource.com/36839
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
gopherbot pushed a commit that referenced this issue Mar 16, 2017
@cherrymui
cmd/compile: clean up SSA-building code
1b85300 
Now that the write barrier insertion is moved to SSA, the SSA
building code can be simplified.

Updates #17583.

Change-Id: I5cacc034b11aa90b0abe6f8dd97e4e3994e2bc25
Reviewed-on: https://go-review.googlesource.com/36840
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
@aclements
Copy link
Member Author

@cherrymui, is there anything left to do on this?

I see we still have needwritebarrier in the front end for determining if a Node has a call. It would be nice to eliminate this duplication, though I don't know how feasible that is.

@bradfitz bradfitz modified the milestones: Go1.9, Go1.9Early May 3, 2017
@aclements
Copy link
Member Author

@cherrymui, ping.

@cherrymui
Copy link
Member

Sorry, I missed it.

Yeah, there is still needwritebarrier in the front end for determining whether we need a call. I'm not sure what the best way is here.

The actual write barrier insertion is already happening in SSA. I'm going to close this.

@cherrymui
Copy link
Member

I tried CL https://go-review.googlesource.com/c/43640/ that simply delete the front end's needwritebarrier. Interestingly, it seems everything works well, and no much performance hit.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/43640 mentions this issue: cmd/compile: remove needwritebarrier from the front end

gopherbot pushed a commit that referenced this issue Oct 16, 2017
@cherrymui
cmd/compile: remove needwritebarrier from the frontend
e0111bb 
The write barrier insertion has moved to the SSA backend's
writebarrier pass. There is still needwritebarrier function
left in the frontend. This function is used in two places:

- fncall, which is called in ascompatet, which is called in
  walking OAS2FUNC. For OAS2FUNC, in order pass we've already
  created temporaries, and there is no write barrier for the
  assignments of these temporaries.

- updateHasCall, which updates the HasCall flag of a node. the
  HasCall flag is then used in
  - fncall, mentioned above.
  - ascompatet. As mentioned above, this is an assignment to
    a temporary, no write barrier.
  - reorder1, which is always called with a list produced by
    ascompatte, which is a list of assignments to stack, which
    have no write barrier.
  - vmatch1, which is called in oaslit with r.Op as OSTRUCTLIT,
    OARRAYLIT, OSLICELIT, or OMAPLIT. There is no write barrier
    in those literals.

Therefore, the needwritebarrier function is unnecessary. This
CL removes it.

Passes "toolstash -cmp" on std cmd.

Updates #17583.

Change-Id: I4b87ba8363d6583e4282a9e607a9ec8ce3ab124a
Reviewed-on: https://go-review.googlesource.com/43640
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
@golang golang locked and limited conversation to collaborators Sep 22, 2018
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.9
Development

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