code review 6821052: cmd/5g, cmd/6g, cmd/8g: remove width check for componentgen.

cmd/5g, cmd/6g, cmd/8g: remove width check for componentgen.

The move to 64-bit ints in 6g made componentgen ineffective.
In componentgen, the code already selects which values it can handle.

On amd64:
benchmark                 old ns/op    new ns/op    delta
BenchmarkBinaryTree17    9477970000   9582314000   +1.10%
BenchmarkFannkuch11      5928750000   5255080000  -11.36%
BenchmarkGobDecode         37103040     31451120  -15.23%
BenchmarkGobEncode         16042490     16844730   +5.00%
BenchmarkGzip             811337400    741373600   -8.62%
BenchmarkGunzip           197928700    192844500   -2.57%
BenchmarkJSONEncode       224164100    140064200  -37.52%
BenchmarkJSONDecode       258346800    231829000  -10.26%
BenchmarkMandelbrot200      7561780      7601615   +0.53%
BenchmarkParse             12970340     11624360  -10.38%
BenchmarkRevcomp         1969917000   1699137000  -13.75%
BenchmarkTemplate         296182000    263117400  -11.16%

[remyoudompheng, 2012-10-31 05:28:49]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

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

[nigeltao, 2012-10-31 06:29:45]

https://codereview.appspot.com/6821052/diff/3001/src/cmd/6g/cgen.c
File src/cmd/6g/cgen.c (right):

https://codereview.appspot.com/6821052/diff/3001/src/cmd/6g/cgen.c#newcode1257
src/cmd/6g/cgen.c:1257: if(w <= 24)
Would s/24/widthptr + 2*widthint/ be better?

[remyoudompheng, 2012-10-31 08:06:07]

Hello nigeltao@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[remyoudompheng, 2012-10-31 08:06:37]

https://codereview.appspot.com/6821052/diff/3001/src/cmd/6g/cgen.c
File src/cmd/6g/cgen.c (right):

https://codereview.appspot.com/6821052/diff/3001/src/cmd/6g/cgen.c#newcode1257
src/cmd/6g/cgen.c:1257: if(w <= 24)
It's just as easy to remove that condition.

[dave_cheney.net, 2012-10-31 08:15:18]

https://codereview.appspot.com/6821052/diff/7001/src/cmd/6g/ggen.c
File src/cmd/6g/ggen.c (right):

https://codereview.appspot.com/6821052/diff/7001/src/cmd/6g/ggen.c#newcode1031
src/cmd/6g/ggen.c:1031: // Special treatment for small types (slices,
strings...)
I don't really understand this comment. Would it make more sense as 

// Treat small types (slices, strings...) specially by (insert statement).

// possibly some comment to indicate this which are not small (complex128 ?)

[DMorsing, 2012-10-31 08:19:44]

https://codereview.appspot.com/6821052/diff/7001/src/cmd/6g/ggen.c
File src/cmd/6g/ggen.c (right):

https://codereview.appspot.com/6821052/diff/7001/src/cmd/6g/ggen.c#newcode1031
src/cmd/6g/ggen.c:1031: // Special treatment for small types (slices,
strings...)
I think you don't need the comment. The one over the function definition gives
enough context as to what's going on.

[nigeltao, 2012-10-31 11:54:08]

If you're removing the w condition, do the same for 8g.

As for commentary, I think the comment on the function defintion is enough.
However, I would change "copy a structure component by component" to "copy a
slice, string or interface value", since it's confusing how a Go slice is
represented by a C struct.

[remyoudompheng, 2012-10-31 20:10:42]

Hello nigeltao@golang.org, dave@cheney.net, daniel.morsing@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[nigeltao, 2012-11-01 01:02:30]

LGTM.

https://codereview.appspot.com/6821052/diff/10001/src/cmd/5g/cgen.c
File src/cmd/5g/cgen.c (right):

https://codereview.appspot.com/6821052/diff/10001/src/cmd/5g/cgen.c#newcode1499
src/cmd/5g/cgen.c:1499: * Slices, strings, interfaces are supported.
s/strings, interfaces/strings and interfaces/ and obviously the same for 6g/8g.

[dave_cheney.net, 2012-11-01 07:19:44]

Looks fine on 5g. I was concerned that the w == 8 w == 12 logic was removed and
would cause alignment issues, but non are apparent.

[remyoudompheng, 2012-11-01 13:36:14]

*** Submitted as http://code.google.com/p/go/source/detail?r=9f8542052bd7 ***

cmd/5g, cmd/6g, cmd/8g: remove width check for componentgen.

The move to 64-bit ints in 6g made componentgen ineffective.
In componentgen, the code already selects which values it can handle.

On amd64:
benchmark                 old ns/op    new ns/op    delta
BenchmarkBinaryTree17    9477970000   9582314000   +1.10%
BenchmarkFannkuch11      5928750000   5255080000  -11.36%
BenchmarkGobDecode         37103040     31451120  -15.23%
BenchmarkGobEncode         16042490     16844730   +5.00%
BenchmarkGzip             811337400    741373600   -8.62%
BenchmarkGunzip           197928700    192844500   -2.57%
BenchmarkJSONEncode       224164100    140064200  -37.52%
BenchmarkJSONDecode       258346800    231829000  -10.26%
BenchmarkMandelbrot200      7561780      7601615   +0.53%
BenchmarkParse             12970340     11624360  -10.38%
BenchmarkRevcomp         1969917000   1699137000  -13.75%
BenchmarkTemplate         296182000    263117400  -11.16%

R=nigeltao, dave, daniel.morsing
CC=golang-dev
http://codereview.appspot.com/6821052