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: eliminate all (some?) convT2{I,E} calls #15375

Open
randall77 opened this issue Apr 19, 2016 · 5 comments
Open

cmd/compile: eliminate all (some?) convT2{I,E} calls #15375

randall77 opened this issue Apr 19, 2016 · 5 comments

Comments

@randall77
Copy link
Contributor

@randall77 randall77 commented Apr 19, 2016

For pointer-shaped types, convT2{I,E} are already done by the compiler. We currently call into the runtime only for non-pointer-shaped types.
There are two cases, where the interface escapes and where it doesn't.

type T struct {
    a, b, c int
}
func f(t T) interface{} {
    return t
}
func g(t T) {
    h(t)
}
//go:noescape
func h(interface{})

In both cases, I think, it would be easier to inline the work that convT2{I,E} does. f does this:

LEAQ    type."".T(SB), AX
MOVQ    AX, (SP)
LEAQ    "".autotmp_0+40(SP), AX
MOVQ    AX, 8(SP)
MOVQ    $0, 16(SP)
CALL    runtime.convT2E(SB)
MOVQ    24(SP), CX
MOVQ    32(SP), DX

instead it could do:

LEAQ    type."".T(SB), AX
MOVQ    AX, (SP)
CALL    runtime.newobject(SB)
MOVQ    8(SP), DX
MOVQ    "".autotmp_0+40(SP), AX
MOVQ    "".autotmp_0+48(SP), BX
MOVQ    "".autotmp_0+56(SP), CX
MOVQ    AX, (DX)
MOVQ    BX, 8(DX)
MOVQ    CX, 16(DX)
LEAQ    type."".T(SB), CX

11 instructions instead of 8, but several runtime calls bypassed (convT2E, plus the typedmemmove it calls, and everything that calls, ...).
The expanded code gets larger if T has a pointer in it. Maybe we use typedmemmove instead of explicit copy instructions in that case.

g is even easier because there is no runtime call required at all. Old code:

LEAQ    type."".T(SB), AX
MOVQ    AX, (SP)
LEAQ    "".autotmp_2+64(SP), AX
MOVQ    AX, 8(SP)
LEAQ    "".autotmp_3+40(SP), AX
MOVQ    AX, 16(SP)
CALL    runtime.convT2E(SB)
MOVQ    24(SP), CX
MOVQ    32(SP), DX

new code:

MOVQ    "".autotmp_2+64(SP), AX
MOVQ    "".autotmp_2+72(SP), BX
MOVQ    "".autotmp_2+80(SP), CX
MOVQ    AX, "".autotmp_3+40(SP)
MOVQ    BX, "".autotmp_3+48(SP)
MOVQ    CX, "".autotmp_3+56(SP)
LEAQ    type."".T(SB), CX
LEAQ    "".autotmp_3+40(SP), DX

one less instruction, and if we can avoid the copy (if autotmp_2 is never modified after), it could be even better. And no write barriers are required on the copy even if T has pointers.

Maybe we do the no-escape optimization first. It seems like an obvious win. That would allow removing the third arg to convT2{I,E}. Then we could think about the escape optimization.

@walken-google

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 18, 2016

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

Loading

gopherbot pushed a commit that referenced this issue Sep 19, 2016
No point in calling a function when we can build the interface
using a known type (or itab) and the address of a local.

Get rid of third arg (preallocated stack space) to convT2{I,E}.

Makes go binary smaller by 0.2%

benchmark                   old ns/op     new ns/op     delta
BenchmarkEfaceInteger-8     16.7          10.1          -39.52%

Update #17118
Update #15375

Change-Id: I9724a1f802bfa1e3957bf1856b55558278e198a2
Reviewed-on: https://go-review.googlesource.com/29373
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@rsc
Copy link
Contributor

@rsc rsc commented Oct 21, 2016

Sounds like the escape case is for later.

Loading

@rsc rsc added this to the Go1.9Early milestone Oct 21, 2016
@rsc rsc removed this from the Go1.8 milestone Oct 21, 2016
@bradfitz bradfitz removed this from the Go1.9Early milestone May 3, 2017
@bradfitz bradfitz added this to the Go1.10Early milestone May 3, 2017
@bradfitz bradfitz added this to the Go1.10Early milestone May 3, 2017
@bradfitz bradfitz removed this from the Go1.9Early milestone May 3, 2017
@bradfitz bradfitz removed this from the Go1.10Early milestone Jun 14, 2017
@bradfitz bradfitz added this to the Go1.10 milestone Jun 14, 2017
@bradfitz bradfitz added this to the Go1.10 milestone Jun 14, 2017
@bradfitz bradfitz removed this from the Go1.10Early milestone Jun 14, 2017
@griesemer griesemer removed this from the Go1.10 milestone Nov 29, 2017
@griesemer griesemer added this to the Go1.11 milestone Nov 29, 2017
@griesemer
Copy link
Contributor

@griesemer griesemer commented Nov 29, 2017

Seems too late now. Moving to 1.11.

Loading

@bradfitz bradfitz removed this from the Go1.11 milestone May 18, 2018
@bradfitz bradfitz added this to the Go1.12 milestone May 18, 2018
@bradfitz bradfitz removed this from the Go1.12 milestone May 18, 2018
@bradfitz bradfitz added this to the Unplanned milestone May 18, 2018
@josharian
Copy link
Contributor

@josharian josharian commented Feb 1, 2020

@randall77 I think this may be done. Do you see anything left to do here?

Loading

@randall77
Copy link
Contributor Author

@randall77 randall77 commented Feb 1, 2020

Yes, I think there's more to be done. The escape case (f in my example) seems nonoptimal. It makes a temporary, then passes the address of that temporary to convT2Enoptr.
We could just call newobject directly, then initialize its contents in compiled code. I don't think it would be any bigger (for pointerless types, anyway).

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants