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

Issue 6686044: code review 6686044: cmd/5g: copy MOVB peephole elimination from 5c (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years ago by dfc
Modified:
9 years ago
Reviewers:
minux1, rsc, remyoudompheng
CC:
golang-dev
Visibility:
Public.

Description

cmd/5g: copy MOVB peephole elimination from 5c The original MOVB elimination code from 5g (commented out) was too agressive. The 5c version is less agressive, but results in moderate improvements. benchmark old ns/op new ns/op delta BenchmarkBinaryTree17 52398590000 51876617000 -1.00% BenchmarkFannkuch11 33195678000 33081147000 -0.35% BenchmarkGobDecode 116363550 113923650 -2.10% BenchmarkGobEncode 55769660 54918220 -1.53% BenchmarkGzip 5464050000 5462128000 -0.04% BenchmarkGunzip 1060395000 1058777000 -0.15% BenchmarkJSONEncode 729699600 721032800 -1.19% BenchmarkJSONDecode 1724457000 1702515000 -1.27% BenchmarkMandelbrot200 45710440 45667740 -0.09% BenchmarkParse 58709720 58497940 -0.36% BenchmarkRevcomp 134762500 129064900 -4.23% BenchmarkTemplate 1881347000 1831787000 -2.63% benchmark old MB/s new MB/s speedup BenchmarkGobDecode 6.60 6.74 1.02x BenchmarkGobEncode 13.76 13.98 1.02x BenchmarkGzip 3.55 3.55 1.00x BenchmarkGunzip 18.30 18.33 1.00x BenchmarkJSONEncode 2.66 2.69 1.01x BenchmarkJSONDecode 1.13 1.14 1.01x BenchmarkParse 0.99 0.99 1.00x BenchmarkRevcomp 18.86 19.69 1.04x BenchmarkTemplate 1.03 1.06 1.03x

Patch Set 1 #

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

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

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -32 lines) Patch
M src/cmd/5g/peep.c View 1 1 chunk +31 lines, -32 lines 2 comments Download

Messages

Total messages: 7
dfc
Hello remyoudompheng@gmail.com, minux.ma@gmail.com, rsc@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
9 years ago (2012-10-13 01:29:15 UTC) #1
remyoudompheng
http://codereview.appspot.com/6686044/diff/1002/src/cmd/5g/peep.c File src/cmd/5g/peep.c (right): http://codereview.appspot.com/6686044/diff/1002/src/cmd/5g/peep.c#newcode138 src/cmd/5g/peep.c:138: */ why did this comment move? the EOR case ...
9 years ago (2012-10-13 08:01:57 UTC) #2
remyoudompheng
Did you find an actual case where these kinds of self-moves were happening? It seems ...
9 years ago (2012-10-13 08:13:37 UTC) #3
dfc
This is a direct c&p of the 5c version for consistency. On 13/10/2012, at 19:01, ...
9 years ago (2012-10-13 08:26:28 UTC) #4
minux1
On Sat, Oct 13, 2012 at 4:26 PM, Dave Cheney <dave@cheney.net> wrote: > This is ...
9 years ago (2012-10-13 08:44:25 UTC) #5
dfc
No problem. I'll repurpose tomorrow. On 13/10/2012, at 19:44, minux <minux.ma@gmail.com> wrote: > > On ...
9 years ago (2012-10-13 09:04:26 UTC) #6
dfc
9 years ago (2012-10-13 23:36:07 UTC) #7
*** Abandoned ***
Sign in to reply to this message.

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