-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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,runtime: compile multiway select statements as switch statements #19331
Comments
A similar return-twice pattern happens for defer. If possible we should do a similar thing for defers. |
@randall77 I think cleaning up deferproc is likely worth doing for similar reasons, but I think the details are distinct enough to track in a separate proposal. |
I like it. Most select cases are small (https://github.com/josharian/countselectcases/blob/master/README.md has some old data), so it'll be a linear not binary search, but I'd wager it'll be imperceptible, particularly since we're eliminating a linear number of function calls. |
I admit that I don't fully understand the reasons why it's done the way it is today, but I support this change, too. In addition to dramatically simplifying some very strange semantics in the runtime, this would eliminate the only use of setcallerpc in the runtime, which has been a thorn in my side before. |
I believe it would eliminate the only use of |
I'm all for this. The suggestion approach seems much cleaner. |
One minor complication I discovered while prototyping this: currently we perform race instrumentation within selectgo using the caller PCs recorded by select{send,recv,default}. This shows up when We could continue instrumenting within selectgo, but it would mean any races would point to the entire select statement, rather than the individual case. Alternatively, we can have the compiler insert instrumentation around the selectgo call, so they can have appropriate line numbers and precise race reports. This would amount to a raceread call for each send case, and just rewriting |
CL https://golang.org/cl/37661 mentions this issue. |
CL 37661 gets rid of the setjmp/longjmp control flow, but keeps the selectfoo calls in place for the purpose of race instrumentation. Post-walk, the AST now looks like:
Further simplifications are still possible. |
If this makes things better for you, great. I can see select getting better from this. I am not as sure about being able to do the same for deferreturn. That will be harder due to stack frame layouts. |
This commit reworks multiway select statements to use normal control flow primitives instead of the previous setjmp/longjmp-like behavior. This simplifies liveness analysis and should prevent issues around "returns twice" function calls within SSA passes. test/live.go is updated because liveness analysis's CFG is more representative of actual control flow. The case bodies are the only real successors of the selectgo call, but previously the selectsend, selectrecv, etc. calls were included in the successors list too. Updates #19331. Change-Id: I7f879b103a4b85e62fc36a270d812f54c0aa3e83 Reviewed-on: https://go-review.googlesource.com/37661 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Closing as the core issue (eliminating setjmp/longjmp in select) is fixed. |
It also occurs to me that deferreturn has basically none of the control flow issues that select did. In effect deferreturn just backs up the PC by 1 to cause itself to run again as a simple way to cause a loop. But to any control flow analysis of the code involved, that's indistinguishable from deferreturn containing a loop and only returning once. So it probably isn't worth changing. |
The issue with defer for me is not with deferreturn, it is with deferproc returning twice (once when first called, again when an panic occurs and that defer recovers). |
Currently, code like:
gets rewritten by cmd/compile into:
The select{send,recv,recv2,default} functions always return false the first time they're called, but internally they save the caller's PC into &sel. runtime.selectgo never returns; instead it waits for a channel operation that can succeed, and then returns to the appropriate PC, behaving as though the function call returned twice.
(To make a C analogy, select{send,recv,recv2,default} are like setjmp, and selectgo is like longjmp.)
This proposal is to instead compile it as (something like):
Pros:
The returns-twice and returns-never logic complicates the CFG. For example, the liveness analysis pass needs to traverse these implicit edges by recognizing runtime.selectfoo function calls. It has also caused bugs in SSA optimizations (for example https://go-review.googlesource.com/#/c/37376/).
gccgo already does this according to @ianlancetaylor
I wouldn't be surprised if the compiler is able to more efficiently initialize the select data structure as a straight forward composite literal, than as a bunch of runtime calls.
We can eliminate a few fields. For example, hselect.ncase and scase.{pc,so}. Potentially more simplifications.
Cons:
@ianlancetaylor @randall77 @rsc @aclements
The text was updated successfully, but these errors were encountered: