Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(19724)

Issue 107930044: code review 107930044: [release-branch.go1.3] cmd/gc: two escape analysis fixes (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 10 months ago by rsc
Modified:
9 years, 10 months ago
Reviewers:
r
CC:
golang-codereviews, r, bradfitz, iant
Visibility:
Public.

Description

[release-branch.go1.3] cmd/gc: two escape analysis fixes ««« CL 108860043 / f153208c0a0e cmd/gc: fix escape analysis for &x inside switch x := v.(type) The analysis for &x was using the loop depth on x set during x's declaration. A type switch creates a list of implicit declarations that were not getting initialized with loop depths. Fixes issue 8176. LGTM=iant R=iant CC=golang-codereviews https://codereview.appspot.com/108860043 »»» ««« CL 108870044 / 331dbd4a6334 cmd/gc: fix &result escaping into result There is a hierarchy of location defined by loop depth: -1 = the heap 0 = function results 1 = local variables (and parameters) 2 = local variable declared inside a loop 3 = local variable declared inside a loop inside a loop etc In general if an address from loopdepth n is assigned to something in loop depth m < n, that indicates an extended lifetime of some form that requires a heap allocation. Function results can be local variables too, though, and so they don't actually fit into the hierarchy very well. Treat the address of a function result as level 1 so that if it is written back into a result, the address is treated as escaping. Fixes issue 8185 . LGTM=iant R=iant CC=golang-codereviews https://codereview.appspot.com/108870044 »»»

Patch Set 1 #

Patch Set 2 : diff -r 96c74cf6dc10 https://code.google.com/p/go/ #

Patch Set 3 : diff -r 8fc565eea9a0 https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+47 lines, -2 lines) Patch
M src/cmd/gc/esc.c View 1 2 chunks +25 lines, -2 lines 0 comments Download
M test/escape2.go View 1 1 chunk +22 lines, -0 lines 0 comments Download

Messages

Total messages: 3
rsc
Hello golang-codereviews@googlegroups.com (cc: bradfitz, iant), I'd like you to review this change to https://code.google.com/p/go/
9 years, 10 months ago (2014-06-11 18:40:39 UTC) #1
r
LGTM
9 years, 10 months ago (2014-06-11 20:51:38 UTC) #2
rsc
9 years, 10 months ago (2014-06-11 21:00:16 UTC) #3
*** Submitted as https://code.google.com/p/go/source/detail?r=7e3382e674d3 ***

[release-branch.go1.3] cmd/gc: two escape analysis fixes

««« CL 108860043 / f153208c0a0e
cmd/gc: fix escape analysis for &x inside switch x := v.(type)

The analysis for &x was using the loop depth on x set
during x's declaration. A type switch creates a list of
implicit declarations that were not getting initialized
with loop depths.

Fixes issue 8176.

LGTM=iant
R=iant
CC=golang-codereviews
https://codereview.appspot.com/108860043
»»»

««« CL 108870044 / 331dbd4a6334
cmd/gc: fix &result escaping into result

There is a hierarchy of location defined by loop depth:

        -1 = the heap
        0 = function results
        1 = local variables (and parameters)
        2 = local variable declared inside a loop
        3 = local variable declared inside a loop inside a loop
        etc

In general if an address from loopdepth n is assigned to
something in loop depth m < n, that indicates an extended
lifetime of some form that requires a heap allocation.

Function results can be local variables too, though, and so
they don't actually fit into the hierarchy very well.
Treat the address of a function result as level 1 so that
if it is written back into a result, the address is treated
as escaping.

Fixes  issue 8185 .

LGTM=iant
R=iant
CC=golang-codereviews
https://codereview.appspot.com/108870044
»»»

LGTM=r
R=golang-codereviews, r
CC=bradfitz, golang-codereviews, iant
https://codereview.appspot.com/107930044
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b