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

Issue 7944043: code review 7944043: cmd/gc: recognize (a.b[0]<<1 | a.b[0]>>31) as a rotate,... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years ago by nigeltao
Modified:
10 years, 12 months ago
Reviewers:
CC:
rsc, iant, remyoudompheng, golang-dev, jcb
Visibility:
Public.

Description

cmd/gc: recognize (a.b[0]<<1 | a.b[0]>>31) as a rotate, not just (x<<1 | x>>31). Fixes issue 5084. On the SHA3 benchmark proposals at https://codereview.appspot.com/7760044/ benchmark old ns/op new ns/op delta BenchmarkPermutationFunction 1288 1191 -7.53% BenchmarkSingleByteWrite 5795 5811 +0.28% BenchmarkBlockWrite512 178 179 +0.56% BenchmarkBlockWrite384 230 233 +1.30% BenchmarkBlockWrite256 282 286 +1.42% BenchmarkBlockWrite224 301 306 +1.66% BenchmarkBulkHashSHA3_512 326885 304548 -6.83% BenchmarkBulkHashSHA3_384 234839 220074 -6.29% BenchmarkBulkHashSHA3_256 186969 175790 -5.98% BenchmarkBulkHashSHA3_224 178133 167489 -5.98% For a function like func g() { x = a[3]<<20 | a[3]>>12 } the asm goes from 0006 (main.go:10) TEXT g+0(SB),$0-0 0007 (main.go:10) MOVL a+12(SB),BP 0008 (main.go:10) LOCALS ,$0 0009 (main.go:11) MOVL BP,BX 0010 (main.go:11) SHLL $20,BX 0011 (main.go:11) SHRL $12,BP 0012 (main.go:11) ORL BP,BX 0013 (main.go:11) MOVL BX,x+0(SB) 0014 (main.go:12) RET , to 0006 (main.go:10) TEXT g+0(SB),$0-0 0007 (main.go:10) LOCALS ,$0 0008 (main.go:11) MOVL a+12(SB),BX 0009 (main.go:11) ROLL $20,BX 0010 (main.go:11) MOVL BX,x+0(SB) 0011 (main.go:12) RET ,

Patch Set 1 #

Patch Set 2 : diff -r ff779e477085 https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r ff779e477085 https://go.googlecode.com/hg/ #

Patch Set 4 : diff -r ff779e477085 https://go.googlecode.com/hg/ #

Total comments: 10

Patch Set 5 : diff -r ff779e477085 https://go.googlecode.com/hg/ #

Patch Set 6 : diff -r 7d736eaa75da https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -8 lines) Patch
M src/cmd/gc/walk.c View 1 2 3 4 5 1 chunk +23 lines, -8 lines 0 comments Download

Messages

Total messages: 9
nigeltao
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com, jcb@google.com), I'd like you to review this change to https://go.googlecode.com/hg/
11 years ago (2013-03-21 10:54:53 UTC) #1
nigeltao
It's been a while since I've written any cmd/gc code. I might be doing something ...
11 years ago (2013-03-21 10:58:11 UTC) #2
rsc
https://codereview.appspot.com/7944043/diff/6002/src/cmd/gc/walk.c File src/cmd/gc/walk.c (right): https://codereview.appspot.com/7944043/diff/6002/src/cmd/gc/walk.c#newcode2893 src/cmd/gc/walk.c:2893: int rmatch; add blank line after decls. you can ...
11 years ago (2013-03-21 20:08:39 UTC) #3
nigeltao
https://codereview.appspot.com/7944043/diff/6002/src/cmd/gc/walk.c File src/cmd/gc/walk.c (right): https://codereview.appspot.com/7944043/diff/6002/src/cmd/gc/walk.c#newcode2898 src/cmd/gc/walk.c:2898: return (a == b) || (strcmp(a->sym->name, b->sym->name) == 0); ...
11 years ago (2013-03-21 23:38:23 UTC) #4
rsc
https://codereview.appspot.com/7944043/diff/6002/src/cmd/gc/walk.c File src/cmd/gc/walk.c (right): https://codereview.appspot.com/7944043/diff/6002/src/cmd/gc/walk.c#newcode2898 src/cmd/gc/walk.c:2898: return (a == b) || (strcmp(a->sym->name, b->sym->name) == 0); ...
11 years ago (2013-03-22 01:21:42 UTC) #5
nigeltao
PTAL.
11 years ago (2013-03-22 23:29:34 UTC) #6
dfc
ping
10 years, 12 months ago (2013-04-01 12:52:28 UTC) #7
remyoudompheng
LGTM
10 years, 12 months ago (2013-04-01 22:55:09 UTC) #8
nigeltao
10 years, 12 months ago (2013-04-02 10:14:47 UTC) #9
*** Submitted as https://code.google.com/p/go/source/detail?r=fa935e01b0a7 ***

cmd/gc: recognize (a.b[0]<<1 | a.b[0]>>31) as a rotate, not just
(x<<1 | x>>31).

Fixes issue 5084.

On the SHA3 benchmark proposals at
https://codereview.appspot.com/7760044/

benchmark                       old ns/op    new ns/op    delta
BenchmarkPermutationFunction         1288         1191   -7.53%
BenchmarkSingleByteWrite             5795         5811   +0.28%
BenchmarkBlockWrite512                178          179   +0.56%
BenchmarkBlockWrite384                230          233   +1.30%
BenchmarkBlockWrite256                282          286   +1.42%
BenchmarkBlockWrite224                301          306   +1.66%
BenchmarkBulkHashSHA3_512          326885       304548   -6.83%
BenchmarkBulkHashSHA3_384          234839       220074   -6.29%
BenchmarkBulkHashSHA3_256          186969       175790   -5.98%
BenchmarkBulkHashSHA3_224          178133       167489   -5.98%

For a function like

func g() {
        x = a[3]<<20 | a[3]>>12
}

the asm goes from

0006 (main.go:10) TEXT    g+0(SB),$0-0
0007 (main.go:10) MOVL    a+12(SB),BP
0008 (main.go:10) LOCALS  ,$0
0009 (main.go:11) MOVL    BP,BX
0010 (main.go:11) SHLL    $20,BX
0011 (main.go:11) SHRL    $12,BP
0012 (main.go:11) ORL     BP,BX
0013 (main.go:11) MOVL    BX,x+0(SB)
0014 (main.go:12) RET     ,

to

0006 (main.go:10) TEXT    g+0(SB),$0-0
0007 (main.go:10) LOCALS  ,$0
0008 (main.go:11) MOVL    a+12(SB),BX
0009 (main.go:11) ROLL    $20,BX
0010 (main.go:11) MOVL    BX,x+0(SB)
0011 (main.go:12) RET     ,

R=rsc, iant, remyoudompheng
CC=golang-dev, jcb
https://codereview.appspot.com/7944043
Sign in to reply to this message.

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