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.

Sign up for GitHub

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

Jump to bottom

cmd/compile: eliminate more dead stores #25132

Open
TocarIP opened this issue Apr 27, 2018 · 6 comments
Open

cmd/compile: eliminate more dead stores #25132

TocarIP opened this issue Apr 27, 2018 · 6 comments
Labels
binary-size compiler/runtime Issues related to the Go compiler and/or runtime. Performance
Milestone
Unplanned

Comments

@TocarIP
Copy link
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 cmd/compile eliminate more cmd/compile: eliminate more dead stores Apr 27, 2018
@josharian
Copy link
Contributor

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

@bcmills bcmills added this to the Unplanned milestone Apr 27, 2018
@mvdan
Copy link
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
Copy link
Contributor

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
Copy link

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

@josharian
Copy link
Contributor

cc @mundaym

@randall77 randall77 mentioned this issue Jan 23, 2019
Closed
@mundaym
Copy link
Member

mundaym commented Jan 23, 2019

Just a quick note: while I was looking at reproducing #29892 I noticed that the LocalAddr and InlMark ops aren't specially handled by the dead store pass, so it will treat them as loads (preventing dead stores being eliminated if their mem arg is used as an input to one of the new ops). Possibly worth fixing if we revisit dead store elimination.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
binary-size compiler/runtime Issues related to the Go compiler and/or runtime. Performance
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
7 participants
@bradfitz @josharian @mvdan @bcmills @gopherbot @mundaym @TocarIP