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 more dead stores #25132

Open
TocarIP opened this Issue Apr 27, 2018 · 5 comments

Comments

Projects
None yet
5 participants
@TocarIP
Contributor

TocarIP commented Apr 27, 2018

What version of Go are you using (go version)?

Trunk
go version devel +3470321 Tue Apr 24 15:26:21 2018 -0500 linux/amd64

What did you do?

go build following program:

package test

//go:noinline
func f(x ...interface{}) {
}

func g() {
        var x string
        f(&x)
}

When looking into generated code I see that autotmp_1 is zeroed and than immediately overwritten

v27    00003 (8)       XORPS   X0, X0
 v7     00004 (8)       MOVUPS  X0, "".x-32(SP)
 v13    00005 (9)       MOVUPS  X0, ""..autotmp_1-16(SP) // zeroing
 v23    00006 (9)       LEAQ    type.*string(SB), AX
 v35    00007 (9)       MOVQ    AX, ""..autotmp_1-16(SP) // followed by 2  stores
 v34    00008 (9)       LEAQ    "".x-32(SP), AX
 v21    00009 (9)       MOVQ    AX, ""..autotmp_1-8(SP)
 v36    00010 (9)       LEAQ    ""..autotmp_1-16(SP), AX

dse pass already eliminates dead stores, including zeroing, so it should also handle this case.

@TocarIP TocarIP changed the title from cmd/compile eliminate more to cmd/compile: eliminate more dead stores Apr 27, 2018

@josharian

This comment has been minimized.

Contributor

josharian commented Apr 27, 2018

cc @mvdan, who recently poked at the deadstore pass.

@bcmills bcmills added the Performance label Apr 27, 2018

@bcmills bcmills added this to the Unplanned milestone Apr 27, 2018

@mvdan

This comment has been minimized.

Member

mvdan commented Apr 29, 2018

@josharian thank you for the ping! Quite obviously, I'm not going to submit anything for DSE in 1.11 :) Will try to get something working during the freeze. Michael Munday lives really close, so I might bribe him with a beer to help me hack on DSE.

@josharian

This comment has been minimized.

Contributor

josharian commented Apr 29, 2018

I poked at this briefly yesterday. I think (but am not sure) that there are two issues here.

  1. Clearing a [1]T vs clearing a T. I think Michael’s recent struct CL might help. We should check.

  2. DSE needs identical—like same v.ID—store dests. But decompose introduces new OffPtr Values for each decomposition. Doing CSE again before DSE would help but that is way too big a hammer. Unwrapping OffPtrs during DSE might do it, or having decompose somehow do in-flight CSE.

I leave this in your hands. :)

@gopherbot

This comment has been minimized.

gopherbot commented Apr 29, 2018

Change https://golang.org/cl/110121 mentions this issue: cmd/compile: unwrap OpOffPtr during DSE

@josharian

This comment has been minimized.

Contributor

josharian commented Apr 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment