code review 28460043: crypto/sha256: block implementation in amd64 assembly (issue 28460043)

[js...@google.com]

Reviewers: agl1,

Message:
Hello a...@golang.org (cc: golan...@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
crypto/sha256: block implementation in amd64 assembly

Benchmark on Intel(R) Xeon(R) CPU X5650  @ 2.67GHz

benchmark old ns/op new ns/op delta
BenchmarkHash8Bytes 1250 743 -40.56%
BenchmarkHash1K 14523 6896 -52.52%
BenchmarkHash8K 106772 51031 -52.21%

benchmark old MB/s new MB/s speedup
BenchmarkHash8Bytes 6.40 10.77 1.68x
BenchmarkHash1K 70.51 148.48 2.11x
BenchmarkHash8K 76.72 160.53 2.09x

Please review this at https://codereview.appspot.com/28460043/

Affected files (+280, -5 lines):
M src/pkg/crypto/sha256/sha256.go
M src/pkg/crypto/sha256/sha256block.go
A src/pkg/crypto/sha256/sha256block_amd64.s
A src/pkg/crypto/sha256/sha256block_decl.go

[a...@golang.org]

LGTM with nits.

Just to confirm - the addition of |s| to |digest| is just to avoid stack
allocation in the block function, right?


https://codereview.appspot.com/28460043/diff/60001/src/pkg/crypto/sha256/sha256.go
File src/pkg/crypto/sha256/sha256.go (right):

https://codereview.appspot.com/28460043/diff/60001/src/pkg/crypto/sha256/sha256.go#newcode49
src/pkg/crypto/sha256/sha256.go:49: type schedule struct {
The use of a new type for this seems excessive.

https://codereview.appspot.com/28460043/