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: pointless static copy code #17113

Closed
josharian opened this issue Sep 15, 2016 · 2 comments
Closed

cmd/compile: pointless static copy code #17113

josharian opened this issue Sep 15, 2016 · 2 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Performance ToolSpeed
Milestone

Comments

@josharian
Copy link
Contributor

package main

type I int

var (
    i int
    x = I(i)
)

compiles to:

".init t=1 size=72 args=0x0 locals=0x0
    0x0000 00000 (issue17111.go:17) TEXT    "".init(SB), $0-0
    0x0000 00000 (issue17111.go:17) MOVQ    (TLS), CX
    0x0009 00009 (issue17111.go:17) CMPQ    SP, 16(CX)
    0x000d 00013 (issue17111.go:17) JLS 65
    0x000f 00015 (issue17111.go:17) FUNCDATA    $0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
    0x000f 00015 (issue17111.go:17) FUNCDATA    $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
    0x000f 00015 (issue17111.go:17) MOVBLZX "".initdone·(SB), AX
    0x0016 00022 (issue17111.go:17) CMPB    AL, $1
    0x0018 00024 (issue17111.go:17) JLS $0, 27
    0x001a 00026 (issue17111.go:17) RET
    0x001b 00027 (issue17111.go:17) JNE $0, 36
    0x001d 00029 (issue17111.go:17) PCDATA  $0, $0
    0x001d 00029 (issue17111.go:17) CALL    runtime.throwinit(SB)
    0x0022 00034 (issue17111.go:17) UNDEF
    0x0024 00036 (issue17111.go:17) MOVB    $1, "".initdone·(SB)
    0x002b 00043 (issue17111.go:13) MOVQ    "".i(SB), AX
    0x0032 00050 (issue17111.go:13) MOVQ    AX, "".x(SB)
    0x0039 00057 (issue17111.go:17) MOVB    $2, "".initdone·(SB)
    0x0040 00064 (issue17111.go:17) RET
    0x0041 00065 (issue17111.go:17) NOP
    0x0041 00065 (issue17111.go:17) CALL    runtime.morestack_noctxt(SB)
    0x0046 00070 (issue17111.go:17) JMP 0
    0x0000 65 48 8b 0c 25 00 00 00 00 48 3b 61 10 76 32 0f  eH..%....H;a.v2.
    0x0010 b6 05 00 00 00 00 3c 01 76 01 c3 75 07 e8 00 00  ......<.v..u....
    0x0020 00 00 0f 0b c6 05 00 00 00 00 01 48 8b 05 00 00  ...........H....
    0x0030 00 00 48 89 05 00 00 00 00 c6 05 00 00 00 00 02  ..H.............
    0x0040 c3 e8 00 00 00 00 eb b8                          ........
    rel 5+4 t=15 TLS+0
    rel 18+4 t=14 "".initdone·+0
    rel 30+4 t=7 runtime.throwinit+0
    rel 38+4 t=14 "".initdone·+-1
    rel 46+4 t=14 "".i+0
    rel 53+4 t=14 "".x+0
    rel 59+4 t=14 "".initdone·+-1
    rel 66+4 t=7 runtime.morestack_noctxt+0
gclocals·33cdeccccebe80329f1fdbee7f5874cb t=9 dupok size=8
    0x0000 01 00 00 00 00 00 00 00                          ........
go.info."".init t=47 size=27
    0x0000 02 22 22 2e 69 6e 69 74 00 00 00 00 00 00 00 00  ."".init........
    0x0010 00 00 00 00 00 00 00 00 00 01 00                 ...........
    rel 9+8 t=1 "".init+0
    rel 17+8 t=1 "".init+72
"".i t=34 size=8
"".x t=34 size=8
"".initdone· t=34 size=1
"".init·f t=9 dupok size=8
    0x0000 00 00 00 00 00 00 00 00                          ........
    rel 0+8 t=1 "".init+0
runtime.gcbits.01 t=9 dupok size=1
    0x0000 01                                               .
type..namedata.*main.I. t=9 dupok size=10
    0x0000 01 00 07 2a 6d 61 69 6e 2e 49                    ...*main.I
type.*"".I t=9 size=56
    0x0000 08 00 00 00 00 00 00 00 08 00 00 00 00 00 00 00  ................
    0x0010 d3 75 f9 8b 00 08 08 36 00 00 00 00 00 00 00 00  .u.....6........
    0x0020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    0x0030 00 00 00 00 00 00 00 00                          ........
    rel 24+8 t=1 runtime.algarray+80
    rel 32+8 t=1 runtime.gcbits.01+0
    rel 40+4 t=5 type..namedata.*main.I.+0
    rel 48+8 t=1 type."".I+0
go.typelink.*"".I t=9 size=4
    0x0000 00 00 00 00                                      ....
    rel 0+4 t=5 type.*"".I+0
runtime.gcbits. t=9 dupok size=0
type."".I t=9 size=64
    0x0000 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    0x0010 d1 cd b2 de 07 08 08 82 00 00 00 00 00 00 00 00  ................
    0x0020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    0x0030 00 00 00 00 00 00 00 00 10 00 00 00 00 00 00 00  ................
    rel 24+8 t=1 runtime.algarray+80
    rel 32+8 t=1 runtime.gcbits.+0
    rel 40+4 t=5 type..namedata.*main.I.+0
    rel 44+4 t=5 type.*"".I+0
    rel 48+4 t=5 type..importpath."".+0

Observe that we generate init code to copy i to x, even though x could be entirely staticly initialized.

@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Sep 15, 2016
staticassign unwraps all CONVNOPs.
However, in the included test, we need the
CONVNOP for everything to typecheck.
Stop unwrapping unnecessarily.

The code we generate for this example is
suboptimal, but that's not new; see #17113.

Fixes #17111.

Change-Id: I29532787a074a6fe19a5cc53271eb9c84bf1b576
Reviewed-on: https://go-review.googlesource.com/29213
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
josharian added a commit to josharian/go that referenced this issue Sep 15, 2016
staticassign unwraps all CONVNOPs.
However, in the included test, we need the
CONVNOP for everything to typecheck.
Stop unwrapping unnecessarily.

The code we generate for this example is
suboptimal, but that's not new; see golang#17113.

Fixes golang#17111.

Change-Id: I29532787a074a6fe19a5cc53271eb9c84bf1b576
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 10, 2016
@rsc rsc modified the milestones: Go1.9, Go1.8Maybe Oct 20, 2016
@josharian josharian modified the milestones: Go1.10, Go1.9 May 2, 2017
@bradfitz bradfitz modified the milestones: Go1.10, Go1.11 Nov 28, 2017
@bradfitz bradfitz modified the milestones: Go1.11, Unplanned May 18, 2018
@agnivade
Copy link
Contributor

This appears fixed with latest tip (go version devel +498eaee461 Fri Nov 15 02:31:58 2019 +0000 linux/amd64)

"".init STEXT nosplit size=15 args=0x0 locals=0x0
	0x0000 00000 (zmq_bench.go:7)	TEXT	"".init(SB), NOSPLIT|ABIInternal, $0-0
	0x0000 00000 (zmq_bench.go:7)	PCDATA	$0, $-2
	0x0000 00000 (zmq_bench.go:7)	PCDATA	$1, $-2
	0x0000 00000 (zmq_bench.go:7)	FUNCDATA	$0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
	0x0000 00000 (zmq_bench.go:7)	FUNCDATA	$1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
	0x0000 00000 (zmq_bench.go:7)	FUNCDATA	$2, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
	0x0000 00000 (zmq_bench.go:7)	PCDATA	$0, $0
	0x0000 00000 (zmq_bench.go:7)	PCDATA	$1, $0
	0x0000 00000 (zmq_bench.go:7)	MOVQ	"".i(SB), AX
	0x0007 00007 (zmq_bench.go:7)	MOVQ	AX, "".x(SB)
	0x000e 00014 (zmq_bench.go:7)	RET
	0x0000 48 8b 05 00 00 00 00 48 89 05 00 00 00 00 c3     H......H.......

I will go ahead and close this. Please feel free to reopen if this was a mistake.

@golang golang locked and limited conversation to collaborators Nov 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Performance ToolSpeed
Projects
None yet
Development

No branches or pull requests

6 participants