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

cmd/compile: pass pointer-shaped receivers using context register #44827

mdempsky opened this issue Mar 6, 2021 · 2 comments

cmd/compile: pass pointer-shaped receivers using context register #44827

mdempsky opened this issue Mar 6, 2021 · 2 comments
binary-size NeedsDecision Performance


Copy link

@mdempsky mdempsky commented Mar 6, 2021

TL;DR: We should look into passing pointer-shaped receivers using the context register.

Today, receiver parameters are effectively simply prepended to the parameter list. This was convenient when Go only had method expressions (i.e., expressions like T.M, where T is a type), because the same function code could be used for both method calls and method expressions.

However, Go later added method values (i.e., expressions like x.M, where x is a value), which require generating wrapper functions and also requires rather complex trampoline code within package reflect. This complexity is needed to adapt between the function closure calling convention (which passes the closure pointer using a dedicated context register) and the method calling convention (which passes receivers using a regular parameter).

If we change the internal calling convention so that receiver arguments are passed using the context register too, then we'd be able to avoid generating method value wrappers. We'd instead need just a single simple, universal wrapper, which could be used by both the compiler and package reflect. (For methods with non-pointer-shaped receivers, we would wrap the interface-calling-convention wrapper for the method, which always uses a pointer-shaped receiver parameter.)

Another benefit (pointed out by @dr2chase) is this would free up an extra register for passing arguments to pointer-receiver methods.

The only downside I see at the moment is that method expressions for pointer-shaped receiver types (e.g., (*T).M) would now need compiler-generated wrappers instead. However, the mitigating factors here are: (1) method expressions can only appear in source so we can always statically generate them (unlike for method values); (2) method expressions seem much less commonly used than method values (caveat: based on my anecdotal experience); and (3) it would be easy to reuse wrappers for methods with the same GC shape.

/cc @aclements @dr2chase @mknyszek

@mdempsky mdempsky added Performance NeedsDecision binary-size labels Mar 6, 2021
@mdempsky mdempsky added this to the Go1.17 milestone Mar 6, 2021
Copy link
Member Author

@mdempsky mdempsky commented Mar 8, 2021

There was a question about what the universal method value wrapper would look like. My idea was something like:

type methodValueWrapper {
    entryPC     uintptr
    recv        unsafe.Pointer
    realEntryPC uintptr

and then have a wrapper function like:

TEXT universalWrapper(SB), NOSPLIT, $0
    MOVQ 16(DX), AX  // load realEntryPC
    MOVQ 8(DX), DX   // load recv into context register
    JMP AX           // jump to underlying method entrypoint

(AX can be replaced with any other register that better fits the regabi calling convention.)

@mdempsky mdempsky removed this from the Go1.17 milestone Apr 29, 2021
@mdempsky mdempsky added this to the Go1.18 milestone Apr 29, 2021
Copy link

@mknyszek mknyszek commented Nov 10, 2021

Since we're in the 1.18 freeze and there hasn't been any movement here, I'm going to place this in the backlog. IIUC this is still something we'd like to do, but unless anyone explicitly plans on working on it for next release, I'm reluctant to put it in the 1.19 milestone.

@mdempsky @aclements Feel free to move it around as necessary.

@mknyszek mknyszek removed this from the Go1.18 milestone Nov 10, 2021
@mknyszek mknyszek added this to the Backlog milestone Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
binary-size NeedsDecision Performance
None yet

No branches or pull requests

2 participants