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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years ago by dfc
Modified:
1 year, 12 months ago
Reviewers:
zhai
CC:
rsc, minux, 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
2 years ago #1
dfc
Hello rsc@golang.org, minux.ma@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
2 years ago #2
minux
LGTM. Some tiny format suggestions below, but this CL is fine without them. Also consider ...
2 years ago #3
dfc
Hello rsc@golang.org, minux.ma@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
2 years ago #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, ...
2 years ago #5
minux
LGTM.
2 years ago #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, ...
2 years ago #7
dfc
Thank you for your comments Russ. In the MOVW foo+4(FP), R0 form, foo is just ...
1 year, 12 months ago #8
rsc
On Mon, Apr 23, 2012 at 17:02, <dave@cheney.net> wrote: > In the MOVW foo+4(FP), R0 ...
1 year, 12 months ago #9
dfc
Hello rsc@golang.org, minux.ma@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
1 year, 12 months ago #10
dfc
If there are no objections I would like to submit this tonight and move on ...
1 year, 12 months ago #11
dfc
Btw. I think I found a small bug in the bytes benchmarks which is causing ...
1 year, 12 months ago #12
rsc
LGTM
1 year, 12 months ago #13
dfc
*** Submitted as http://code.google.com/p/go/source/detail?r=b15b7aaca931 *** bytes: add assembly version of IndexByte for ARM benchmark old ...
1 year, 12 months ago #14
zhai
1 year, 12 months ago #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 1278:e6ce13d99bf5