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

Issue 119360043: code review 119360043: runtime: convert hash functions to Go calling convention. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 7 months ago by khr
Modified:
10 years, 7 months ago
Reviewers:
gobot, rsc, adonovan, dvyukov
CC:
golang-codereviews, dvyukov, dave_cheney.net, khr1
Visibility:
Public.

Description

runtime: convert hash functions to Go calling convention. Create proper closures so hash functions can be called directly from Go. Rearrange calling convention so return value is directly accessible.

Patch Set 1 #

Patch Set 2 : diff -r b3d45fe62d73 https://khr%40golang.org@code.google.com/p/go/ #

Patch Set 3 : diff -r b3d45fe62d73 https://khr%40golang.org@code.google.com/p/go/ #

Patch Set 4 : diff -r b3d45fe62d73 https://khr%40golang.org@code.google.com/p/go/ #

Patch Set 5 : diff -r b3d45fe62d73 https://khr%40golang.org@code.google.com/p/go/ #

Patch Set 6 : diff -r b3d45fe62d73 https://khr%40golang.org@code.google.com/p/go/ #

Patch Set 7 : diff -r b3d45fe62d73 https://khr%40golang.org@code.google.com/p/go/ #

Total comments: 2

Patch Set 8 : diff -r b3d45fe62d73 https://khr%40golang.org@code.google.com/p/go/ #

Patch Set 9 : diff -r c6b7f7454ffc https://khr%40golang.org@code.google.com/p/go/ #

Patch Set 10 : diff -r c6b7f7454ffc https://khr%40golang.org@code.google.com/p/go/ #

Patch Set 11 : diff -r c6b7f7454ffc https://khr%40golang.org@code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+199 lines, -419 lines) Patch
M src/cmd/gc/reflect.c View 2 chunks +7 lines, -2 lines 0 comments Download
M src/cmd/gc/subr.c View 8 chunks +41 lines, -34 lines 0 comments Download
M src/pkg/runtime/alg.goc View 9 chunks +46 lines, -175 lines 0 comments Download
M src/pkg/runtime/asm_386.s View 3 chunks +22 lines, -54 lines 0 comments Download
M src/pkg/runtime/asm_amd64.s View 3 chunks +22 lines, -54 lines 0 comments Download
M src/pkg/runtime/asm_amd64p32.s View 1 chunk +3 lines, -33 lines 0 comments Download
M src/pkg/runtime/asm_arm.s View 1 chunk +3 lines, -33 lines 0 comments Download
M src/pkg/runtime/export_test.go View 1 chunk +0 lines, -6 lines 0 comments Download
M src/pkg/runtime/hashmap.go View 8 chunks +8 lines, -8 lines 0 comments Download
M src/pkg/runtime/hashmap_fast.go View 6 chunks +6 lines, -6 lines 0 comments Download
M src/pkg/runtime/iface.goc View 1 chunk +3 lines, -4 lines 0 comments Download
M src/pkg/runtime/runtime.h View 2 chunks +14 lines, -10 lines 0 comments Download
M src/pkg/runtime/stubs.go View 1 chunk +24 lines, -0 lines 0 comments Download

Messages

Total messages: 11
khr
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://khr%40golang.org@code.google.com/p/go/
10 years, 7 months ago (2014-07-30 22:11:41 UTC) #1
dvyukov
LGTM https://codereview.appspot.com/119360043/diff/120001/src/pkg/runtime/asm_amd64.s File src/pkg/runtime/asm_amd64.s (right): https://codereview.appspot.com/119360043/diff/120001/src/pkg/runtime/asm_amd64.s#newcode963 src/pkg/runtime/asm_amd64.s:963: MOVQ 8(SP), AX // ptr to string struct ...
10 years, 7 months ago (2014-07-31 11:28:09 UTC) #2
dave_cheney.net
On 31 Jul 2014 21:28, "dvyukov via golang-codereviews" < golang-codereviews@googlegroups.com> wrote: > > LGTM > ...
10 years, 7 months ago (2014-07-31 11:30:47 UTC) #3
dvyukov
On Thu, Jul 31, 2014 at 3:30 PM, Dave Cheney <dave@cheney.net> wrote: > > On ...
10 years, 7 months ago (2014-07-31 11:41:53 UTC) #4
khr1
On Thu, Jul 31, 2014 at 4:28 AM, <dvyukov@google.com> wrote: > LGTM > > > ...
10 years, 7 months ago (2014-07-31 21:29:50 UTC) #5
khr
*** Submitted as https://code.google.com/p/go/source/detail?r=2aa6648d7146 *** runtime: convert hash functions to Go calling convention. Create proper ...
10 years, 7 months ago (2014-07-31 22:07:08 UTC) #6
gobot
This CL appears to have broken the linux-amd64-race builder. See http://build.golang.org/log/2839ba46f393bb31cb34c71f621afcb63913f0a2
10 years, 7 months ago (2014-07-31 22:07:49 UTC) #7
khr1
Whoops, CL is missing a file. Fixed. On Thu, Jul 31, 2014 at 3:07 PM, ...
10 years, 7 months ago (2014-07-31 22:13:39 UTC) #8
adonovan
On 2014/07/31 22:13:39, khr1 wrote: > Whoops, CL is missing a file. Fixed. > > ...
10 years, 7 months ago (2014-08-05 21:59:30 UTC) #9
dave_cheney.net
I can confirm that the time to run the tests in the go.tools repo have ...
10 years, 7 months ago (2014-08-05 22:12:39 UTC) #10
rsc
10 years, 7 months ago (2014-08-06 18:15:36 UTC) #11
Message was sent while issue was closed.
The problem is that ifacehash is returning 0 as the hash code for a non-empty
interface value. CL 125720044 should make this better.
Sign in to reply to this message.

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