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

Issue 6106044: code review 6106044: bytes: add assembly version of IndexByte for ARM (Closed)

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

Description

bytes: add assembly version of IndexByte for ARM benchmark old ns/op new ns/op delta BenchmarkIndexByte32 459 126 -72.55% BenchmarkIndexByte4K 52404 10939 -79.13% BenchmarkIndexByte4M 54470800 11177370 -79.48% BenchmarkIndexByte64M 1010803000 178860500 -82.31% benchmark old MB/s new MB/s speedup BenchmarkIndexByte32 69.58 252.63 3.63x BenchmarkIndexByte4K 78.16 374.42 4.79x BenchmarkIndexByte4M 77.00 375.25 4.87x BenchmarkIndexByte64M 66.39 375.20 5.65x

Patch Set 1 #

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

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

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

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

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

Total comments: 6

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

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

Total comments: 12

Patch Set 9 : diff -r 69a5c6d983b4 https://code.google.com/p/go #

Patch Set 10 : diff -r 69a5c6d983b4 https://code.google.com/p/go #

Patch Set 11 : diff -r 69a5c6d983b4 https://code.google.com/p/go #

Patch Set 12 : diff -r 29665c4bae69 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -3 lines) Patch
M src/pkg/bytes/asm_arm.s View 1 2 3 4 5 6 7 8 9 1 chunk +22 lines, -3 lines 0 comments Download

Messages

Total messages: 15
dfc
Hello rsc@golang.org, minux.ma@gmail.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
11 years, 11 months ago (2012-04-21 13:04:18 UTC) #1
dfc
Hello rsc@golang.org, minux.ma@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
11 years, 11 months ago (2012-04-21 13:41:49 UTC) #2
minux1
LGTM. Some tiny format suggestions below, but this CL is fine without them. Also consider ...
11 years, 11 months ago (2012-04-21 14:15:53 UTC) #3
dfc
Hello rsc@golang.org, minux.ma@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
11 years, 11 months ago (2012-04-21 14:26:48 UTC) #4
dfc
Thank you for your prompt review. http://codereview.appspot.com/6106044/diff/11001/src/pkg/bytes/asm_arm.s File src/pkg/bytes/asm_arm.s (right): http://codereview.appspot.com/6106044/diff/11001/src/pkg/bytes/asm_arm.s#newcode9 src/pkg/bytes/asm_arm.s:9: ADD R0, R1, ...
11 years, 11 months ago (2012-04-21 14:27:00 UTC) #5
minux1
LGTM.
11 years, 11 months ago (2012-04-21 14:37:02 UTC) #6
rsc
http://codereview.appspot.com/6106044/diff/2003/src/pkg/bytes/asm_arm.s File src/pkg/bytes/asm_arm.s (right): http://codereview.appspot.com/6106044/diff/2003/src/pkg/bytes/asm_arm.s#newcode6 src/pkg/bytes/asm_arm.s:6: MOVW 0(FP), R0 // start Instead of these comments, ...
11 years, 11 months ago (2012-04-23 14:07:41 UTC) #7
dfc
Thank you for your comments Russ. In the MOVW foo+4(FP), R0 form, foo is just ...
11 years, 11 months ago (2012-04-23 21:02:52 UTC) #8
rsc
On Mon, Apr 23, 2012 at 17:02, <dave@cheney.net> wrote: > In the MOVW foo+4(FP), R0 ...
11 years, 11 months ago (2012-04-23 21:08:03 UTC) #9
dfc
Hello rsc@golang.org, minux.ma@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
11 years, 11 months ago (2012-04-23 21:15:34 UTC) #10
dfc
If there are no objections I would like to submit this tonight and move on ...
11 years, 11 months ago (2012-04-23 22:44:03 UTC) #11
dfc
Btw. I think I found a small bug in the bytes benchmarks which is causing ...
11 years, 11 months ago (2012-04-24 11:39:51 UTC) #12
rsc
LGTM
11 years, 11 months ago (2012-04-25 01:37:45 UTC) #13
dfc
*** Submitted as http://code.google.com/p/go/source/detail?r=b15b7aaca931 *** bytes: add assembly version of IndexByte for ARM benchmark old ...
11 years, 11 months ago (2012-04-25 03:18:51 UTC) #14
zhai
11 years, 11 months ago (2012-04-25 04:40:58 UTC) #15
A slight change, will save 2 or 3 cycles each  loop
--------------
 TEXT  ·IndexByte(SB),7,$0
       MOVW    base+0(FP), R0
       MOVW    len+4(FP), R1
       MOVBU   c+12(FP), R2     // byte to find
       MOVW    R1, R4           // store length for later

       TEQ    $0, R1
       B.EQ   _done
       MOVBU.P 1(R0), R3
_loop:
       CMP     R2, R3
       SUB.S.NE   $1, R1
       MOVBU.NE.P 1(R0), R3
       B.NE   _loop

       TEQ     $0, R1
_done:
       MOVW.EQ $-1, R0
       SUB.NE  R1, R4, R0
       MOVW    R0, index+16(FP)
       RET


On Wed, Apr 25, 2012 at 11:18 AM, <dave@cheney.net> wrote:

> *** Submitted as
>
http://code.google.com/p/go/**source/detail?r=b15b7aaca931<http://code.google...
>
>
> bytes: add assembly version of IndexByte for ARM
>
> benchmark                        old ns/op    new ns/op    delta
> BenchmarkIndexByte32                   459          126  -72.55%
> BenchmarkIndexByte4K                 52404        10939  -79.13%
> BenchmarkIndexByte4M              54470800     11177370  -79.48%
> BenchmarkIndexByte64M           1010803000    178860500  -82.31%
>
> benchmark                         old MB/s     new MB/s  speedup
> BenchmarkIndexByte32                 69.58       252.63    3.63x
> BenchmarkIndexByte4K                 78.16       374.42    4.79x
> BenchmarkIndexByte4M                 77.00       375.25    4.87x
> BenchmarkIndexByte64M                66.39       375.20    5.65x
>
> R=rsc, minux.ma
> CC=golang-dev
>
http://codereview.appspot.com/**6106044<http://codereview.appspot.com/6106044>
>
>
>
http://codereview.appspot.com/**6106044/<http://codereview.appspot.com/6106044/>
>
Sign in to reply to this message.

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