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

Issue 7289048: code review 7289048: cmd/gc: add way to specify 'noescape' for extern funcs (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 1 month ago by rsc
Modified:
11 years, 1 month ago
Reviewers:
CC:
golang-dev, dfc, minux1, DMorsing, remyoudompheng, adg, agl1, iant
Visibility:
Public.

Description

cmd/gc: add way to specify 'noescape' for extern funcs A new comment directive //go:noescape instructs the compiler that the following external (no body) func declaration should be treated as if none of its arguments escape to the heap. Fixes issue 4099.

Patch Set 1 #

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

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

Patch Set 4 : diff -r 1f343d81deab https://code.google.com/p/go/ #

Patch Set 5 : diff -r 1f343d81deab https://code.google.com/p/go/ #

Patch Set 6 : diff -r ec4982dfc0fc https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+257 lines, -145 lines) Patch
M src/cmd/gc/doc.go View 1 2 3 4 3 chunks +26 lines, -1 line 0 comments Download
M src/cmd/gc/esc.c View 1 2 3 4 5 2 chunks +15 lines, -4 lines 0 comments Download
M src/cmd/gc/go.h View 1 2 3 2 chunks +2 lines, -0 lines 0 comments Download
M src/cmd/gc/go.y View 1 2 chunks +4 lines, -0 lines 0 comments Download
M src/cmd/gc/lex.c View 1 2 chunks +22 lines, -8 lines 0 comments Download
M src/cmd/gc/y.tab.c View 1 46 chunks +136 lines, -132 lines 0 comments Download
M test/escape2.go View 1 2 3 4 5 1 chunk +26 lines, -0 lines 0 comments Download
A test/fixedbugs/issue4099.go View 1 1 chunk +26 lines, -0 lines 0 comments Download

Messages

Total messages: 22
rsc
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go/
11 years, 1 month ago (2013-02-03 20:16:39 UTC) #1
dfc
Neat. I'll try some benchmarks today! On Mon, Feb 4, 2013 at 7:16 AM, <rsc@golang.org> ...
11 years, 1 month ago (2013-02-03 20:19:25 UTC) #2
minux1
i'm a little worried about the trend of introducing annotations like this (previously //go:fieldtrack is ...
11 years, 1 month ago (2013-02-03 20:23:48 UTC) #3
dfc
> people will ask for more.... (for example, //go:noboundcheck, > //go:expecttrue [mirror gcc's __builtin_expect]). > ...
11 years, 1 month ago (2013-02-03 20:42:24 UTC) #4
minux1
On Monday, February 4, 2013, Dave Cheney wrote: > > people will ask for more.... ...
11 years, 1 month ago (2013-02-03 20:54:49 UTC) #5
rsc
This is just a suggestion. It's the best I could come up with to address ...
11 years, 1 month ago (2013-02-03 20:59:27 UTC) #6
dfc
SGTM. Lets try some benchmarks and see if this pays for itself. On 04/02/2013, at ...
11 years, 1 month ago (2013-02-03 21:13:07 UTC) #7
DMorsing
One of the ideas I had during my brainstorming session with Dave was the idea ...
11 years, 1 month ago (2013-02-03 21:22:57 UTC) #8
remyoudompheng
On 2013/02/03 21:22:57, DMorsing wrote: > One of the ideas I had during my brainstorming ...
11 years, 1 month ago (2013-02-03 21:25:35 UTC) #9
DMorsing
On 2013/02/03 21:25:35, remyoudompheng wrote: > This looks dangerous exposed as is. It should be ...
11 years, 1 month ago (2013-02-03 21:41:28 UTC) #10
adg
I'm quite happy with the comments for this purpose because if a compiler doesn't support ...
11 years, 1 month ago (2013-02-04 00:17:37 UTC) #11
agl1
Benchmarks generally have a for loop to call the same function in a loop, so ...
11 years, 1 month ago (2013-02-04 16:34:04 UTC) #12
iant
I think this is as reasonable an approach an any. We need to know this ...
11 years, 1 month ago (2013-02-04 17:17:57 UTC) #13
rsc
Hello golang-dev@googlegroups.com, dave@cheney.net, minux.ma@gmail.com, daniel.morsing@gmail.com, remyoudompheng@gmail.com, adg@golang.org, agl@golang.org, iant@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
11 years, 1 month ago (2013-02-05 03:49:04 UTC) #14
rsc
I added some words to the docs for cmd/gc.
11 years, 1 month ago (2013-02-05 03:49:28 UTC) #15
iant
LGTM
11 years, 1 month ago (2013-02-05 05:58:10 UTC) #16
dfc
Something is confusing me, hg tip, unmodified, already says % go build -gcflags -m | ...
11 years, 1 month ago (2013-02-05 09:57:44 UTC) #17
rsc
Yes, it looks like -m is printing the wrong thing for external functions, but otherwise ...
11 years, 1 month ago (2013-02-05 11:41:33 UTC) #18
dfc
Fair enough, should I raise an issue for -m ? I'm also have problems determining ...
11 years, 1 month ago (2013-02-05 11:46:58 UTC) #19
rsc
On Tue, Feb 5, 2013 at 6:46 AM, Dave Cheney <dave@cheney.net> wrote: > Fair enough, ...
11 years, 1 month ago (2013-02-05 11:52:24 UTC) #20
rsc
Don't bother adding an issue for -m. I'll fix it before submitting this CL.
11 years, 1 month ago (2013-02-05 11:57:03 UTC) #21
rsc
11 years, 1 month ago (2013-02-05 12:00:48 UTC) #22
*** Submitted as https://code.google.com/p/go/source/detail?r=d42bb094c316 ***

cmd/gc: add way to specify 'noescape' for extern funcs

A new comment directive //go:noescape instructs the compiler
that the following external (no body) func declaration should be
treated as if none of its arguments escape to the heap.

Fixes issue 4099.

R=golang-dev, dave, minux.ma, daniel.morsing, remyoudompheng, adg, agl, iant
CC=golang-dev
https://codereview.appspot.com/7289048
Sign in to reply to this message.

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