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

Issue 6674048: code review 6674048: cmd/5g: peep.c: reactivate some optimisations (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 6 months ago by dfc
Modified:
1 year, 5 months ago
Reviewers:
CC:
remyoudompheng, rsc, minux, golang-dev
Visibility:
Public.

Description

cmd/5g: peep.c: reactivate some optimisations

Thanks to Minux and Remy for their advice.

The EOR optimisation is applied to a few places in the stdlib.

// hash/crc32/crc32.go
func update(crc uint32, tab *Table, p []byte) uint32 {
	crc = ^crc
	for _, v := range p {
        	crc = tab[byte(crc)^v] ^ (crc >> 8)
	}
	return ^crc
}

before:

--- prog list "update" ---
0164 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:101) TEXT       
update+0(SB),$12-24
0165 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:101) MOVW        tab+4(FP),R8
0166 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:102) MOVW        crc+0(FP),R0
0167 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:102) EOR         $-1,R0,R5
0168 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:103) MOVW        p+8(FP),R0
0169 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:103) MOVW       
R0,autotmp_0019+-12(SP)

after:

--- prog list "update" ---
0164 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:101) TEXT       
update+0(SB),$12-24
0165 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:101) MOVW        tab+4(FP),R8
0166 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:102) MOVW        crc+0(FP),R0
0167 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:102) MVN         R0,R5
0168 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:103) MOVW        p+8(FP),R0
0169 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:103) MOVW       
R0,autotmp_0019+-12(SP)

After 5l has done its work, 

        crc = ^crc
   3d710:       e59d0014        ldr     r0, [sp, #20]
   3d714:       e3e0b000        mvn     fp, #0
   3d718:       e020500b        eor     r5, r0, fp

becomes

        crc = ^crc
   3d710:       e59d0014        ldr     r0, [sp, #20]
   3d714:       e1e05000        mvn     r5, r0


The MOVB optimisation has a small impact on the stdlib, in strconv
and gzip.

// GZIP (RFC 1952) is little-endian, unlike ZLIB (RFC 1950).
func put2(p []byte, v uint16) {
        p[0] = uint8(v >> 0)
        p[1] = uint8(v >> 8)
}

before:

--- prog list "put2" ---
1369 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:76) TEXT       put2+0(SB),$0-16
1370 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:76) MOVHU      v+12(FP),R4
1371 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVHU      R4,R0
1372 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVHU      R0,R0
1373 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVBU      R0,R1
1374 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVBU      R1,R3
1375 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVW       $p+0(FP),R1

after:

--- prog list "put2" ---
1369 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:76) TEXT       put2+0(SB),$0-16
1370 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:76) MOVHU      v+12(FP),R4
1371 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVHU      R4,R0
1372 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVBU      R0,R1
1373 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVBU      R1,R3
1374 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVW       $p+0(FP),R1

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 #

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

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

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

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

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -17 lines) Patch
M src/cmd/5g/peep.c View 1 2 3 3 chunks +16 lines, -17 lines 0 comments Download

Messages

Total messages: 8
dfc
Hello remyoudompheng@gmail.com, 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
1 year, 6 months ago #1
minux
LGTM.
1 year, 6 months ago #2
minux
the EOR optimization seems to remove a MOVW $-1, R11, if it's the case, please ...
1 year, 6 months ago #3
dfc
On 2012/10/14 09:52:52, minux wrote: > the EOR optimization seems to remove a MOVW $-1, ...
1 year, 6 months ago #4
minux
On 2012/10/14 10:17:13, dfc wrote: > I have updated the description, I haven't been able ...
1 year, 6 months ago #5
dfc
ping
1 year, 5 months ago #6
rsc
LGTM
1 year, 5 months ago #7
dfc
1 year, 5 months ago #8
*** Submitted as http://code.google.com/p/go/source/detail?r=1bd1e8e9aaa8 ***

cmd/5g: peep.c: reactivate some optimisations

Thanks to Minux and Remy for their advice.

The EOR optimisation is applied to a few places in the stdlib.

// hash/crc32/crc32.go
func update(crc uint32, tab *Table, p []byte) uint32 {
	crc = ^crc
	for _, v := range p {
        	crc = tab[byte(crc)^v] ^ (crc >> 8)
	}
	return ^crc
}

before:

--- prog list "update" ---
0164 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:101) TEXT       
update+0(SB),$12-24
0165 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:101) MOVW        tab+4(FP),R8
0166 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:102) MOVW        crc+0(FP),R0
0167 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:102) EOR         $-1,R0,R5
0168 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:103) MOVW        p+8(FP),R0
0169 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:103) MOVW       
R0,autotmp_0019+-12(SP)

after:

--- prog list "update" ---
0164 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:101) TEXT       
update+0(SB),$12-24
0165 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:101) MOVW        tab+4(FP),R8
0166 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:102) MOVW        crc+0(FP),R0
0167 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:102) MVN         R0,R5
0168 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:103) MOVW        p+8(FP),R0
0169 (/home/dfc/go/src/pkg/hash/crc32/crc32.go:103) MOVW       
R0,autotmp_0019+-12(SP)

After 5l has done its work, 

        crc = ^crc
   3d710:       e59d0014        ldr     r0, [sp, #20]
   3d714:       e3e0b000        mvn     fp, #0
   3d718:       e020500b        eor     r5, r0, fp

becomes

        crc = ^crc
   3d710:       e59d0014        ldr     r0, [sp, #20]
   3d714:       e1e05000        mvn     r5, r0


The MOVB optimisation has a small impact on the stdlib, in strconv
and gzip.

// GZIP (RFC 1952) is little-endian, unlike ZLIB (RFC 1950).
func put2(p []byte, v uint16) {
        p[0] = uint8(v >> 0)
        p[1] = uint8(v >> 8)
}

before:

--- prog list "put2" ---
1369 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:76) TEXT       put2+0(SB),$0-16
1370 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:76) MOVHU      v+12(FP),R4
1371 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVHU      R4,R0
1372 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVHU      R0,R0
1373 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVBU      R0,R1
1374 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVBU      R1,R3
1375 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVW       $p+0(FP),R1

after:

--- prog list "put2" ---
1369 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:76) TEXT       put2+0(SB),$0-16
1370 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:76) MOVHU      v+12(FP),R4
1371 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVHU      R4,R0
1372 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVBU      R0,R1
1373 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVBU      R1,R3
1374 (/home/dfc/go/src/pkg/compress/gzip/gzip.go:77) MOVW       $p+0(FP),R1

R=remyoudompheng, rsc, minux.ma
CC=golang-dev
http://codereview.appspot.com/6674048
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1278:e6ce13d99bf5