-
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: skip zero'ing order array for select statements #40399
Comments
Change https://golang.org/cl/244797 mentions this issue: |
Hi! I'm a new contributor to go. I authored a CL as described to address this issue here: https://go-review.googlesource.com/c/go/+/244797 One question I have is, to help me learn the codebase, how does |
@hherman1 Thanks for looking into this. I can review your CL once the tree reopens for Go 1.16 development. As for OAS with nil as the RHS node, look at how gc/ssa.go handles OAS nodes. There's logic there for treating a nil RHS as an implicit zero value. Let me know if you still have questions; I can elaborate in more detail. Edit: This is the main bit of code responsible for handling go/src/cmd/compile/internal/gc/ssa.go Lines 1243 to 1257 in 8696ae8
|
@mdempsky I did try making your suggestion change, that only save us 2 MOVQ instructions, there's still DUFFZERO instruction in generated assembly. |
Thanks for the pointer @mdempsky! Very helpful |
@cuonglm if you don't mind, I'm curious how did you generate/inspect the produced assembly? Would like to try that myself |
Just build the compiler with your patch, then run |
@cuonglm Thanks. To be clear, I don't expect this to be a huge win; just one or two instructions per typical Are you sure the DUFFZERO instruction was zero'ing the |
Ah right, that DUFFZERO is for zero'ing the |
Thanks for confirming. FWIW, #40410 should help reduce (but won't eliminate) the DUFFZERO instructions. |
I'm convinced I'm doing something wrong, as the assembly being generated for me for the following source is the same with and without my patch. I'm taking the following steps:
Anything obvious I'm doing wrong here? Does your process look different than mine? The program in question:
|
@hherman1: they produce different output for me. You also may want to try with just 3 cases, 2 for listen to channel, 1 for default. Something like:
You can easily see there's no |
Is the CL you're using locally the same as the one I posted above? |
Yes, I cherry pick your CL. Before:
After:
|
Change https://golang.org/cl/251517 mentions this issue: |
The compiler zero-initializes the order array before calling
selectgo
, but the runtime ends up overwriting almost all of it anyway. We could probably shave a few instructions from call sites ifselectgo
took full responsibility for initializing the array.I think this is just a matter of adding a
pollorder[0] = 0
assignment inselectgo
, and removing thenod(OAS, order, nil)
assignment in cmd/compile/internal/gc/select.go.See also #40397 (comment).
/cc @cuonglm
The text was updated successfully, but these errors were encountered: