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

Issue 204110043: code review 204110043: compiler: Basic escape analysis for the go frontend. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 1 month ago by cmang
Modified:
9 years ago
Reviewers:
iant
CC:
minux, gofrontend-dev_googlegroups.com
Visibility:
Public.

Description

compiler: Basic escape analysis for the go frontend. This is an implementation of the algorithm described in "Escape Analysis in Java" by Choi et. al. It relies on dataflow information to discover variable references to one another. Handles assignments to closures and association between closures variables and the variables of the enclosing scope. Dataflow analysis does not discover references through range statements e.g. for _, v := range a will not recognize that all values of v are references to a.

Patch Set 1 #

Patch Set 2 : diff -r d42a0819e2eb https://code.google.com/p/gofrontend #

Patch Set 3 : diff -r d42a0819e2eb https://code.google.com/p/gofrontend #

Patch Set 4 : diff -r d42a0819e2eb https://code.google.com/p/gofrontend #

Patch Set 5 : diff -r d42a0819e2eb https://code.google.com/p/gofrontend #

Total comments: 31

Patch Set 6 : diff -r d42a0819e2eb https://code.google.com/p/gofrontend #

Total comments: 14

Patch Set 7 : diff -r d42a0819e2eb https://code.google.com/p/gofrontend #

Total comments: 40

Patch Set 8 : diff -r d42a0819e2eb https://code.google.com/p/gofrontend #

Total comments: 6

Patch Set 9 : diff -r d42a0819e2eb https://code.google.com/p/gofrontend #

Total comments: 6

Patch Set 10 : diff -r d42a0819e2eb https://code.google.com/p/gofrontend #

Total comments: 7

Patch Set 11 : diff -r d42a0819e2eb https://code.google.com/p/gofrontend #

Patch Set 12 : diff -r 2169f7d99472 https://code.google.com/p/gofrontend #

Patch Set 13 : diff -r ee25dd0adff9 https://code.google.com/p/gofrontend #

Patch Set 14 : diff -r ee25dd0adff9 https://code.google.com/p/gofrontend #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2643 lines, -692 lines) Patch
M go/dataflow.cc View 2 chunks +24 lines, -3 lines 0 comments Download
A go/escape.h View 1 chunk +306 lines, -0 lines 0 comments Download
A go/escape.cc View 1 chunk +1167 lines, -0 lines 0 comments Download
M go/expressions.h View 10 chunks +721 lines, -0 lines 0 comments Download
M go/expressions.cc View 18 chunks +109 lines, -634 lines 0 comments Download
M go/go.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M go/gogo.h View 18 chunks +122 lines, -9 lines 0 comments Download
M go/gogo.cc View 6 chunks +78 lines, -6 lines 0 comments Download
M go/statements.h View 9 chunks +83 lines, -0 lines 0 comments Download
M go/statements.cc View 5 chunks +29 lines, -40 lines 0 comments Download

Messages

Total messages: 26
cmang
Hello iant@golang.org (cc: gofrontend-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/gofrontend
9 years, 1 month ago (2015-02-23 19:44:04 UTC) #1
cmang
On 2015/02/23 19:44:04, cmang wrote: > Hello mailto:iant@golang.org (cc: mailto:gofrontend-dev@googlegroups.com), > > I'd like you ...
9 years, 1 month ago (2015-02-23 19:44:35 UTC) #2
minux
On 2015/02/23 19:44:35, cmang wrote: > Diff to gcc/go/Make-lang.in: > > --- a/gcc/go/Make-lang.in > +++ ...
9 years, 1 month ago (2015-02-23 21:02:18 UTC) #3
cmang
On 2015/02/23 21:02:18, minux wrote: > On 2015/02/23 19:44:35, cmang wrote: > > Diff to ...
9 years, 1 month ago (2015-02-23 21:09:27 UTC) #4
iant
A quick first look. https://codereview.appspot.com/204110043/diff/70001/go/dataflow.cc File go/dataflow.cc (right): https://codereview.appspot.com/204110043/diff/70001/go/dataflow.cc#newcode177 go/dataflow.cc:177: // analysis for case statements ...
9 years, 1 month ago (2015-02-24 03:34:50 UTC) #5
cmang
https://codereview.appspot.com/204110043/diff/70001/go/dataflow.cc File go/dataflow.cc (right): https://codereview.appspot.com/204110043/diff/70001/go/dataflow.cc#newcode177 go/dataflow.cc:177: // analysis for case statements here; the analysis for ...
9 years, 1 month ago (2015-02-24 18:35:17 UTC) #6
iant
A few more comments. Still haven't really gone through the whole thing yet. https://codereview.appspot.com/204110043/diff/70001/go/escape.h File ...
9 years, 1 month ago (2015-02-24 19:27:14 UTC) #7
cmang
https://codereview.appspot.com/204110043/diff/90001/go/dataflow.cc File go/dataflow.cc (right): https://codereview.appspot.com/204110043/diff/90001/go/dataflow.cc#newcode193 go/dataflow.cc:193: for (Expression_list::const_iterator p1 = ce->args()->begin(); On 2015/02/24 19:27:14, iant ...
9 years, 1 month ago (2015-02-24 20:55:04 UTC) #8
iant
https://codereview.appspot.com/204110043/diff/110001/go/dataflow.cc File go/dataflow.cc (right): https://codereview.appspot.com/204110043/diff/110001/go/dataflow.cc#newcode173 go/dataflow.cc:173: || statement->thunk_statement() != NULL) Needs a comment about thunk_statement. ...
9 years, 1 month ago (2015-02-24 22:11:15 UTC) #9
cmang
https://codereview.appspot.com/204110043/diff/110001/go/dataflow.cc File go/dataflow.cc (right): https://codereview.appspot.com/204110043/diff/110001/go/dataflow.cc#newcode173 go/dataflow.cc:173: || statement->thunk_statement() != NULL) On 2015/02/24 22:11:14, iant wrote: ...
9 years, 1 month ago (2015-02-25 00:28:50 UTC) #10
iant
https://codereview.appspot.com/204110043/diff/110001/go/go.cc File go/go.cc (right): https://codereview.appspot.com/204110043/diff/110001/go/go.cc#newcode113 go/go.cc:113: // Consider escape analysis information when deciding if a ...
9 years, 1 month ago (2015-02-25 19:35:07 UTC) #11
cmang
https://codereview.appspot.com/204110043/diff/130001/go/gogo.cc File go/gogo.cc (right): https://codereview.appspot.com/204110043/diff/130001/go/gogo.cc#newcode3395 go/gogo.cc:3395: return; On 2015/02/25 19:35:07, iant wrote: > If I ...
9 years, 1 month ago (2015-02-25 22:43:30 UTC) #12
iant
https://codereview.appspot.com/204110043/diff/130001/go/gogo.cc File go/gogo.cc (right): https://codereview.appspot.com/204110043/diff/130001/go/gogo.cc#newcode3395 go/gogo.cc:3395: return; On 2015/02/25 22:43:30, cmang wrote: > On 2015/02/25 ...
9 years, 1 month ago (2015-02-26 18:21:34 UTC) #13
cmang
https://codereview.appspot.com/204110043/diff/130001/go/gogo.cc File go/gogo.cc (right): https://codereview.appspot.com/204110043/diff/130001/go/gogo.cc#newcode3395 go/gogo.cc:3395: return; On 2015/02/26 18:21:33, iant wrote: > On 2015/02/25 ...
9 years, 1 month ago (2015-02-26 18:50:53 UTC) #14
iant
More comments. I'm troubled by the imprecision of resolve_var_reference, because this code must be either ...
9 years, 1 month ago (2015-02-27 21:17:04 UTC) #15
cmang
https://codereview.appspot.com/204110043/diff/170001/go/gogo.cc File go/gogo.cc (right): https://codereview.appspot.com/204110043/diff/170001/go/gogo.cc#newcode3326 go/gogo.cc:3326: resolve_var_reference(Expression* expr) On 2015/02/27 21:17:03, iant wrote: > I'm ...
9 years, 1 month ago (2015-02-27 22:00:52 UTC) #16
iant
https://codereview.appspot.com/204110043/diff/170001/go/gogo.cc File go/gogo.cc (right): https://codereview.appspot.com/204110043/diff/170001/go/gogo.cc#newcode3326 go/gogo.cc:3326: resolve_var_reference(Expression* expr) On 2015/02/27 22:00:52, cmang wrote: > > ...
9 years, 1 month ago (2015-02-27 23:49:57 UTC) #17
cmang
https://codereview.appspot.com/204110043/diff/110001/go/gogo.cc File go/gogo.cc (right): https://codereview.appspot.com/204110043/diff/110001/go/gogo.cc#newcode3201 go/gogo.cc:3201: // A traversal class used to build a call ...
9 years ago (2015-03-09 21:51:16 UTC) #18
cmang
On 2015/02/27 23:49:57, iant wrote: > https://codereview.appspot.com/204110043/diff/170001/go/gogo.cc > File go/gogo.cc (right): > > https://codereview.appspot.com/204110043/diff/170001/go/gogo.cc#newcode3326 > ...
9 years ago (2015-03-09 21:58:09 UTC) #19
iant
The case I'm most worried about in resolve_var_reference is composite literals. Right now I think ...
9 years ago (2015-03-09 22:34:38 UTC) #20
cmang
On 2015/03/09 22:34:38, iant wrote: > The case I'm most worried about in resolve_var_reference is ...
9 years ago (2015-03-09 22:49:06 UTC) #21
cmang
On 2015/03/09 22:49:06, cmang wrote: > On 2015/03/09 22:34:38, iant wrote: > > The case ...
9 years ago (2015-03-12 18:24:14 UTC) #22
iant
LGTM Let's get this in and see what happens next.
9 years ago (2015-03-12 22:47:33 UTC) #23
iant
*** Submitted as https://code.google.com/p/gofrontend/source/detail?r=b0d3b9400c63 *** compiler: Basic escape analysis for the go frontend. This is ...
9 years ago (2015-03-12 22:47:57 UTC) #24
cmang
On 2015/03/12 22:47:33, iant wrote: > LGTM > > Let's get this in and see ...
9 years ago (2015-03-12 22:56:06 UTC) #25
iant
9 years ago (2015-03-12 23:34:33 UTC) #26
Whoops, sorry!
Sign in to reply to this message.

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