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: VARDEF instruction keeps temporary live when it is unused #9059

Closed
DanielMorsing opened this issue Nov 4, 2014 · 2 comments
Closed
Milestone

Comments

@DanielMorsing
Copy link
Contributor

Compile the following program:

package footest

import (
    "bytes"
)

func foo() {
    var buf bytes.Buffer
    var b [4]byte
    buf.Write(b[:])
}

This produces the following assembly for the buf.Write line:

    0x0049 00073 (test.go:10)   MOVQ    $4,DX
    0x0050 00080 (test.go:10)   MOVQ    $4,CX
    0x0057 00087 (test.go:10)   MOVQ    BX,"".autotmp_0002+64(SP)
    0x005c 00092 (test.go:10)   MOVQ    BX,8(SP)
    0x0061 00097 (test.go:10)   MOVQ    DX,"".autotmp_0002+72(SP)
    0x0066 00102 (test.go:10)   MOVQ    DX,16(SP)
    0x006b 00107 (test.go:10)   MOVQ    CX,"".autotmp_0002+80(SP)
    0x0070 00112 (test.go:10)   MOVQ    CX,24(SP)
    0x0075 00117 (test.go:10)   PCDATA  $0,$0
    0x0075 00117 (test.go:10)   CALL    ,bytes.(*Buffer).Write(SB)

Note that after this, autotmp_0002 is never used again.

regopt gets passed this as its input (obtained via -R -v):

00000 (test.go:7)   TEXT    "".foo+0(SB),$0-0
00001 (test.go:7)   FUNCDATA    $0,"".gcargs·0+0(SB)
00002 (test.go:7)   FUNCDATA    $1,"".gclocals·1+0(SB)
00003 (test.go:7)   TYPE    "".b+0(SP),$4
00004 (test.go:7)   TYPE    "".&buf+0(SP),$8
00005 (test.go:7)   TYPE    "".autotmp_0001+0(SP),$8
00006 (test.go:8)   MOVQ    $type.bytes.Buffer+0(SB),BX
00007 (test.go:8)   MOVQ    BX,(SP)
00008 (test.go:8)   CALL    ,runtime.newobject+0(SB)
00009 (test.go:8)   MOVQ    8(SP),BX
00010 (test.go:8)   MOVQ    BX,"".&buf+0(SP)
00011 (test.go:9)   VARDEF  ,"".b+0(SP)
00012 (test.go:9)   LEAQ    "".b+0(SP),BX
00013 (test.go:9)   MOVL    $0,(BX)
00014 (test.go:10)  LEAQ    "".b+0(SP),BX
00015 (test.go:10)  MOVQ    BX,"".autotmp_0001+0(SP)
00016 (test.go:10)  MOVQ    "".&buf+0(SP),BX
00017 (test.go:10)  MOVQ    BX,(SP)
00018 (test.go:10)  MOVQ    "".autotmp_0001+0(SP),BX
00019 (test.go:10)  MOVQ    BX,"".autotmp_0003+0(SP)
00020 (test.go:10)  CHECKNIL    "".autotmp_0003+0(SP),
00021 (test.go:10)  VARDEF  ,"".autotmp_0002+0(SP)
00022 (test.go:10)  MOVQ    $4,"".autotmp_0004+0(SP)
00023 (test.go:10)  MOVQ    $4,"".autotmp_0005+0(SP)
00024 (test.go:10)  MOVQ    "".autotmp_0003+0(SP),BX
00025 (test.go:10)  MOVQ    BX,"".autotmp_0002+0(SP)
00026 (test.go:10)  MOVQ    "".autotmp_0004+0(SP),BX
00027 (test.go:10)  MOVQ    BX,"".autotmp_0002+8(SP)
00028 (test.go:10)  MOVQ    "".autotmp_0005+0(SP),BX
00029 (test.go:10)  MOVQ    BX,"".autotmp_0002+16(SP)
00030 (test.go:10)  MOVQ    "".autotmp_0002+0(SP),BX
00031 (test.go:10)  MOVQ    BX,8(SP)
00032 (test.go:10)  MOVQ    "".autotmp_0002+8(SP),BX
00033 (test.go:10)  MOVQ    BX,16(SP)
00034 (test.go:10)  MOVQ    "".autotmp_0002+16(SP),BX
00035 (test.go:10)  MOVQ    BX,24(SP)
00036 (test.go:10)  CALL    ,bytes.(*Buffer).Write+0(SB)
00037 (test.go:11)  RET ,

Note that autotmp_0002 has a vardef which was inserted during cgen_slice, but no
matching varkill instruction.

Do we need to insert vardefs for slices? We don't care about their cap or len anymore.
@bradfitz bradfitz removed the new label Dec 18, 2014
@rsc rsc added this to the Go1.5Maybe milestone Apr 10, 2015
@rsc rsc removed the repo-main label Apr 14, 2015
@rsc rsc changed the title cmd/gc: VARDEF instruction keeps temporary live when it is unused cmd/compile: VARDEF instruction keeps temporary live when it is unused Jun 8, 2015
@rsc
Copy link
Contributor

rsc commented Jun 29, 2015

Too late for Go 1.5. Hopefully SSA will take care of this. We will see. I don't believe it is VARDEF, though; I think it's leftover stores that were not properly optimized away. Running the optimizer a second time would identify them correctly.

@rsc rsc modified the milestones: Unplanned, Go1.5Maybe Jun 29, 2015
@josharian josharian modified the milestones: Go1.6, Unplanned Jun 29, 2015
@rsc rsc modified the milestones: dev.ssa, Go1.6 Nov 4, 2015
@bradfitz bradfitz modified the milestones: dev.ssa, Go1.7 Mar 2, 2016
@rsc
Copy link
Contributor

rsc commented May 17, 2016

Fixed by SSA.

/cc @randall77

go tool compile -S -ssa=0:

    0x0045 00069 (/tmp/x.go:10) MOVQ    $4, BP
    0x004c 00076 (/tmp/x.go:10) MOVQ    $4, DX
    0x0053 00083 (/tmp/x.go:10) MOVQ    AX, (SP)
    0x0057 00087 (/tmp/x.go:10) MOVQ    CX, "".autotmp_1+64(SP)
    0x005c 00092 (/tmp/x.go:10) MOVQ    CX, 8(SP)
    0x0061 00097 (/tmp/x.go:10) MOVQ    BP, "".autotmp_1+72(SP)
    0x0066 00102 (/tmp/x.go:10) MOVQ    BP, 16(SP)
    0x006b 00107 (/tmp/x.go:10) MOVQ    DX, "".autotmp_1+80(SP)
    0x0070 00112 (/tmp/x.go:10) MOVQ    DX, 24(SP)
    0x0075 00117 (/tmp/x.go:10) PCDATA  $0, $0
    0x0075 00117 (/tmp/x.go:10) CALL    bytes.(*Buffer).Write(SB)

go tool compile -S:

    0x003c 00060 (/tmp/x.go:10) MOVQ    AX, (SP)
    0x0040 00064 (/tmp/x.go:10) LEAQ    "".b+60(SP), AX
    0x0045 00069 (/tmp/x.go:10) MOVQ    AX, 8(SP)
    0x004a 00074 (/tmp/x.go:10) MOVQ    $4, 16(SP)
    0x0053 00083 (/tmp/x.go:10) MOVQ    $4, 24(SP)
    0x005c 00092 (/tmp/x.go:10) PCDATA  $0, $0
    0x005c 00092 (/tmp/x.go:10) CALL    bytes.(*Buffer).Write(SB)

@rsc rsc closed this as completed May 17, 2016
@golang golang locked and limited conversation to collaborators May 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants