-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/compile: writebarrier code not happy with single(ish)-block-loops that contain writebarriers #19067
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
Comments
I have mailed a sequence of CLs for the new writebarrier implementation this morning. The new code doesn't have this problem. |
In particular, CL https://go-review.googlesource.com/c/36834/ is the one most relevant to this issue. |
And if we were to fix the old implementation, yes, Phi function needs to be special-cased in that check. |
CL https://golang.org/cl/36940 mentions this issue. |
CL https://golang.org/cl/36834 mentions this issue. |
The old writebarrier implementation fails to handle single-block loop where a memory Phi value depends on the write barrier store in the same block. The new implementation (CL 36834) doesn't have this problem. Add a test to ensure it. Fix #19067. Change-Id: Iab13c6817edc12be8a048d18699b4450fa7ed712 Reviewed-on: https://go-review.googlesource.com/36940 Reviewed-by: David Chase <[email protected]>
Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (
go version
)?1.9 devel:
go version devel +e2948f7efe Mon Feb 13 20:30:31 2017 +0000 darwin/amd64
What operating system and processor architecture are you using (
go env
)?amd64/darwin
What did you do?
https://play.golang.org/p/72MnkjS4P4
What did you expect to see?
This works in the playground.
What did you see instead?
There's a check in the writebarrier code that might not be necessary, or might need to exclude phi functions:
The offending input looks like this:
The text was updated successfully, but these errors were encountered: