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: unused variable escapes to the heap #27188

Open
marigonzes opened this Issue Aug 24, 2018 · 6 comments

Comments

Projects
None yet
5 participants
@marigonzes

marigonzes commented Aug 24, 2018

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

go version go1.10.3 windows/amd64

What operating system and processor architecture are you using (go env)?

set GOARCH=amd64
set GOBIN=
set GOCACHE=...
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=...
set GORACE=
set GOROOT=C:\Go
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=...

What did you do?

I compiled the following program using `go build -gcflags="-m -m" to check if any of the variables escaped to the heap.
https://play.golang.org/p/iZCz8oXRoi__h

What did you expect to see?

I expected only y to escape.

What did you see instead?

Instead, both x and y escaped to the heap.
In my opinion, only y should escape, because there is no way for x to leave the scope of this function. Alternatively, the assignment of the address of x to k should be entirely removed.

@marigonzes marigonzes changed the title from cmd/compile: to cmd/compile: unused variable escapes to the heap Aug 24, 2018

@ianlancetaylor ianlancetaylor added this to the Go1.12 milestone Aug 24, 2018

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Aug 24, 2018

@dr2chase dr2chase modified the milestones: Go1.12, Unplanned Aug 24, 2018

@dr2chase

This comment has been minimized.

Contributor

dr2chase commented Aug 24, 2018

The algorithm we use is not that clever and just tracks flow by variable name.
If/when we get around to moving escape analysis into the SSA phase (there are several difficult prerequisites in the way of this happening -- and these are planned, e.g., moving call expansion into SSA) this will be fixed as a side-effect.

Since there's an easy workaround and big blockers for the proper fix, I would not commit to fixing this in 1.12.

@rasky

This comment has been minimized.

Member

rasky commented Aug 24, 2018

Do you have a list offhand of all the prerequisites for a new SSA-based escape analysis?

@dr2chase

This comment has been minimized.

Contributor

dr2chase commented Aug 28, 2018

I think (but now I am a little bit less sure, but lack the time right now to be more sure):

  • need generic opcodes to allow us to talk about making things on-stack-or-heap-but-not-sure-yet, to be resolved by escape analysis.
  • calls expanded in SSA itself, not before (I think). At least, calls to newobject, right?
  • need generic opcodes for unexpanded calls.
  • (maybe) an early part of SSA made able to run across some or all of a package, since escape analysis is interprocedural within a package (I am having some second thoughts here, current analysis punts on cycles and thus we could break edges with worst-case punts and do things one function at a time). But we might not always punt on cycles; if we improve the analysis to be able to tell the difference between the spine of a list and the leaves of the list (CDR vs CAR, e.g.) then there will be profit in recursive analysis; we can obtain interesting fixed points (I prototyped this, then realized that inability to tell CAR from CDR meant the benefits were very minor).
  • dealing with shared versus not for things allocated in a stack frame will force a revision of SSA form; anything shared is potentially modified by a call, anything local (that is not address-taken and passed in to call) is not.
@randall77

This comment has been minimized.

Contributor

randall77 commented Aug 28, 2018

In addition to that, I think we need to understand what the benefit would be. It's going to be a lot of work, I only want to undertake it if we expect some commensurate benefits.

@dr2chase

This comment has been minimized.

Contributor

dr2chase commented Aug 30, 2018

One possible benefit, from talking to a guy at GopherCon (@cixel) who's interested in doing instrumentation for taint detection/analysis, is doing dataflow-informed instrumentation. Idea is that we'd transform to SSA, do whatever instrumentation we want (that might, in some cases, have effects on escape analysis -- and might also have effects on stack frames), then run escape analysis etc.

This is still a little hand-wavy. He's looking the code for tsan and msan, and modifying things at the AST level, but having the ability to do flow analysis would be helpful (I wonder if it might not also help tsan and msan)

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