code review 6686044: cmd/5g: copy MOVB peephole elimination from 5c

cmd/5g: copy MOVB peephole elimination from 5c

The original MOVB elimination code from 5g (commented out)
was too agressive.

The 5c version is less agressive, but results in
moderate improvements.

benchmark                 old ns/op    new ns/op    delta
BenchmarkBinaryTree17   52398590000  51876617000   -1.00%
BenchmarkFannkuch11     33195678000  33081147000   -0.35%
BenchmarkGobDecode        116363550    113923650   -2.10%
BenchmarkGobEncode         55769660     54918220   -1.53%
BenchmarkGzip            5464050000   5462128000   -0.04%
BenchmarkGunzip          1060395000   1058777000   -0.15%
BenchmarkJSONEncode       729699600    721032800   -1.19%
BenchmarkJSONDecode      1724457000   1702515000   -1.27%
BenchmarkMandelbrot200     45710440     45667740   -0.09%
BenchmarkParse             58709720     58497940   -0.36%
BenchmarkRevcomp          134762500    129064900   -4.23%
BenchmarkTemplate        1881347000   1831787000   -2.63%

benchmark                  old MB/s     new MB/s  speedup
BenchmarkGobDecode             6.60         6.74    1.02x
BenchmarkGobEncode            13.76        13.98    1.02x
BenchmarkGzip                  3.55         3.55    1.00x
BenchmarkGunzip               18.30        18.33    1.00x
BenchmarkJSONEncode            2.66         2.69    1.01x
BenchmarkJSONDecode            1.13         1.14    1.01x
BenchmarkParse                 0.99         0.99    1.00x
BenchmarkRevcomp              18.86        19.69    1.04x
BenchmarkTemplate              1.03         1.06    1.03x

[dave_cheney.net, 2012-10-13 01:29:15]

Hello remyoudompheng@gmail.com, minux.ma@gmail.com, rsc@golang.org (cc:
golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[remyoudompheng, 2012-10-13 08:01:57]

http://codereview.appspot.com/6686044/diff/1002/src/cmd/5g/peep.c
File src/cmd/5g/peep.c (right):

http://codereview.appspot.com/6686044/diff/1002/src/cmd/5g/peep.c#newcode138
src/cmd/5g/peep.c:138: */
why did this comment move? the EOR case below has nothing to do with it.

http://codereview.appspot.com/6686044/diff/1002/src/cmd/5g/peep.c#newcode168
src/cmd/5g/peep.c:168: continue;
why move these lines? it seems the thing you have corrected is the misplaced r1
= r->link

[remyoudompheng, 2012-10-13 08:13:37]

Did you find an actual case where these kinds of self-moves were
happening? It seems that for the standard case of compiling cmd/go,
the output is exactly identical before and after the patch.

Rémy.

[dave_cheney.net, 2012-10-13 08:26:28]

This is a direct c&p of the 5c version for consistency. 

On 13/10/2012, at 19:01, remyoudompheng@gmail.com wrote:

> 
> http://codereview.appspot.com/6686044/diff/1002/src/cmd/5g/peep.c
> File src/cmd/5g/peep.c (right):
> 
> http://codereview.appspot.com/6686044/diff/1002/src/cmd/5g/peep.c#newcode138
> src/cmd/5g/peep.c:138: */
> why did this comment move? the EOR case below has nothing to do with it.
> 
> http://codereview.appspot.com/6686044/diff/1002/src/cmd/5g/peep.c#newcode168
> src/cmd/5g/peep.c:168: continue;
> why move these lines? it seems the thing you have corrected is the
> misplaced r1 = r->link
> 
> http://codereview.appspot.com/6686044/

[minux1, 2012-10-13 08:44:25]

On Sat, Oct 13, 2012 at 4:26 PM, Dave Cheney <dave@cheney.net> wrote:

> This is a direct c&p of the 5c version for consistency.

I agree with remy that we need to keep the history of 5g/peep.c as clean as
possible,
so please don't move those lines.

In fact, we probably should change 5c/peep.c as well, because the first
comment
block is indeed misplaced.

[dave_cheney.net, 2012-10-13 09:04:26]

No problem. I'll repurpose tomorrow. 

On 13/10/2012, at 19:44, minux <minux.ma@gmail.com> wrote:

> 
> On Sat, Oct 13, 2012 at 4:26 PM, Dave Cheney <dave@cheney.net> wrote:
>> This is a direct c&p of the 5c version for consistency.
> I agree with remy that we need to keep the history of 5g/peep.c as clean as
possible,
> so please don't move those lines.
> 
> In fact, we probably should change 5c/peep.c as well, because the first
comment
> block is indeed misplaced.

[dave_cheney.net, 2012-10-13 23:36:07]

*** Abandoned ***