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

Issue 131450043: code review 131450043: runtime: give nosplit functions 32 more bytes of headroom (Closed)

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

Description

runtime: give nosplit functions 32 more bytes of headroom The Go calling convention uses more stack space than C. On 64-bit systems we've been right up against the limit (128 bytes, so only 16 words) and doing awful things to our source code to work around it. Instead of continuing to do awful things, raise the limit to 160 bytes. I am prepared to raise the limit to 192 bytes if necessary, but I think this will be enough. Should fix current link-time stack overflow errors on - nacl/arm - netbsd/amd64 - openbsd/amd64 - solaris/amd64 - windows/amd64

Patch Set 1 #

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

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -3 lines) Patch
M src/pkg/runtime/stack.h View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/runtime/stack_test.go View 1 1 chunk +2 lines, -1 line 0 comments Download
M test/nosplit.go View 1 2 2 chunks +9 lines, -1 line 0 comments Download

Messages

Total messages: 5
rsc
Hello r (cc: golang-codereviews@googlegroups.com, iant), I'd like you to review this change to https://code.google.com/p/go/
10 years, 6 months ago (2014-08-27 18:08:22 UTC) #1
rsc
*** Submitted as https://code.google.com/p/go/source/detail?r=07a722aece2d *** runtime: give nosplit functions 32 more bytes of headroom The ...
10 years, 6 months ago (2014-08-27 18:08:29 UTC) #2
gobot
This CL appears to have broken the linux-386-387 builder. See http://build.golang.org/log/db979e051ce731ae682e76cb4225f8f8107f3b71
10 years, 6 months ago (2014-08-27 18:34:51 UTC) #3
r
LGTM
10 years, 6 months ago (2014-08-27 18:42:56 UTC) #4
aram
10 years, 6 months ago (2014-08-27 19:04:20 UTC) #5
Message was sent while issue was closed.
LGTM
Sign in to reply to this message.

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