code review 6547064: cmd/8g: don't create redundant temporaries in bgen.

cmd/8g: don't create redundant temporaries in bgen.

Comparisons used to create temporaries for arguments
even if they were already variables or addressable.
Removing the extra ones reduces pressure on regopt.

benchmark                 old ns/op    new ns/op    delta
BenchmarkGobDecode         50787620     49908980   -1.73%
BenchmarkGobEncode         19870190     19473030   -2.00%
BenchmarkGzip            3214321000   3067929000   -4.55%
BenchmarkGunzip           496792800    465828600   -6.23%
BenchmarkJSONEncode       232524800    263864400  +13.48%
BenchmarkJSONDecode       622038400    506600600  -18.56%
BenchmarkMandelbrot200     23937310     45913060  +91.81%
BenchmarkParse             14364450     13997010   -2.56%
BenchmarkRevcomp         6919028000   6480009000   -6.35%
BenchmarkTemplate         594458800    539528200   -9.24%

benchmark                  old MB/s     new MB/s  speedup
BenchmarkGobDecode            15.11        15.38    1.02x
BenchmarkGobEncode            38.63        39.42    1.02x
BenchmarkGzip                  6.04         6.33    1.05x
BenchmarkGunzip               39.06        41.66    1.07x
BenchmarkJSONEncode            8.35         7.35    0.88x
BenchmarkJSONDecode            3.12         3.83    1.23x
BenchmarkParse                 4.03         4.14    1.03x
BenchmarkRevcomp              36.73        39.22    1.07x
BenchmarkTemplate              3.26         3.60    1.10x

[remyoudompheng, 2012-09-23 09:15:45]

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

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

[mtj1, 2012-09-23 09:17:57]

BenchmarkMandelbrot200     23937310     45913060  +91.81%

???

On Sun, Sep 23, 2012 at 5:15 AM,  <remyoudompheng@gmail.com> wrote:
> BenchmarkMandelbrot200     23937310     45913060  +91.81%



-- 
Michael T. Jones | Chief Technology Advocate  | mtj@google.com |  +1
650-335-5765

[remyoudompheng, 2012-09-23 09:26:30]

Some notes:
* the benchmarks were done on a Core Duo Mobile T2300 (1.66GHz). Results are
similar on a Core 2 Quad with GOARCH=386
* the Mandelbrot regression is a mystery to me. I wonder what is the issue but
I'm not sure it is relevant since x87 arithmetic will be removed
* the JSON benchmarks are flaky as usual due to moving stack boundaries.

Some interesting stack size reductions:

Before
< src/pkg/time/format.go:101) TEXT    nextStdChunk+0(SB),$2232-28
< src/pkg/time/format.go:259) TEXT    match+0(SB),$24-20
< src/pkg/time/format.go:288) TEXT    appendUint+0(SB),$208-32
< src/pkg/time/format.go:376) TEXT    Time.Format+0(SB),$1204-32
< src/pkg/time/format.go:642) TEXT    Parse+0(SB),$2012-40
< src/pkg/time/format.go:989) TEXT    ParseDuration+0(SB),$404-24

After
> src/pkg/time/format.go:101) TEXT    nextStdChunk+0(SB),$1684-28
> src/pkg/time/format.go:259) TEXT    match+0(SB),$0-20
> src/pkg/time/format.go:288) TEXT    appendUint+0(SB),$164-32
> src/pkg/time/format.go:376) TEXT    Time.Format+0(SB),$960-32
> src/pkg/time/format.go:642) TEXT    Parse+0(SB),$1608-40
> src/pkg/time/format.go:989) TEXT    ParseDuration+0(SB),$312-24

[remyoudompheng, 2012-09-23 09:30:00]

On 2012/09/23 09:17:57, mtj1 wrote:
> BenchmarkMandelbrot200     23937310     45913060  +91.81%
> 
> ???

I don't understand what the issue is. The code looks exactly identical except
for a simplified loop control and shuffled stack offsets.

[remyoudompheng, 2012-09-23 09:51:03]

Ah, maybe a cache collision:

Now on my Core 2 Quad:

benchmark                 old ns/op    new ns/op    delta
BenchmarkMandelbrot200     19004500     35397380  +86.26%

After adding "var A [48]byte; _ = A" at top of the BenchmarkMandelbrot200
function:

benchmark                 old ns/op    new ns/op    delta
BenchmarkMandelbrot200     13397590     12407690   -7.39%

[DMorsing, 2012-09-23 10:09:32]

FWIW, here are my numbers for a core i5 ivy bridge

benchmark                 old ns/op    new ns/op    delta
BenchmarkBinaryTree17    3258965000   3186988000   -2.21%
BenchmarkFannkuch11      4158159000   3718785000  -10.57%
BenchmarkGobDecode         17433820     16863610   -3.27%
BenchmarkGobEncode          6190964      5851658   -5.48%
BenchmarkGzip            1288106000   1210740000   -6.01%
BenchmarkGunzip           149991800    139396800   -7.06%
BenchmarkJSONEncode        70442140     81534100  +15.75%
BenchmarkJSONDecode       208782900    162309900  -22.26%
BenchmarkMandelbrot200      7435862      7214852   -2.97%
BenchmarkParse              4454478      4320996   -3.00%
BenchmarkRevcomp         1563759000   1385049000  -11.43%
BenchmarkTemplate         178690400    160645200  -10.10%

benchmark                  old MB/s     new MB/s  speedup
BenchmarkGobDecode            44.03        45.51    1.03x
BenchmarkGobEncode           123.98       131.17    1.06x
BenchmarkGzip                 15.06        16.03    1.06x
BenchmarkGunzip              129.37       139.20    1.08x
BenchmarkJSONEncode           27.55        23.80    0.86x
BenchmarkJSONDecode            9.29        11.96    1.29x
BenchmarkParse                13.00        13.40    1.03x
BenchmarkRevcomp             162.54       183.51    1.13x
BenchmarkTemplate             10.86        12.08    1.11x

definitely some weirdness going on here.

[mtj1, 2012-09-23 10:58:24]

No advice. Just intrigued. Strange. Possibly the smaller
function/stack in place A leads to unhappy stack splitting elsewhere?

On Sun, Sep 23, 2012 at 5:30 AM,  <remyoudompheng@gmail.com> wrote:
> On 2012/09/23 09:17:57, mtj1 wrote:
>>
>> BenchmarkMandelbrot200     23937310     45913060  +91.81%
>
>
>> ???
>
>
> I don't understand what the issue is. The code looks exactly identical
> except for a simplified loop control and shuffled stack offsets.
>
> http://codereview.appspot.com/6547064/



-- 
Michael T. Jones | Chief Technology Advocate  | mtj@google.com |  +1
650-335-5765

[remyoudompheng, 2012-09-23 11:10:51]

On 2012/09/23 10:58:24, mtj1 wrote:
> No advice. Just intrigued. Strange. Possibly the smaller
> function/stack in place A leads to unhappy stack splitting elsewhere?

It's not stack splitting: stack splitting would show up in the profile (it
doesn't) and would give a consistent performance impact over architectures (but
DMorsing's numbers contradict that).

perf stat says that there are really less instructions executed, but at 0.3
instructions/cycle instead of 0.6.

[rsc, 2012-09-24 16:28:26]

LGTM

[remyoudompheng, 2012-09-24 19:32:54]

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

cmd/8g: don't create redundant temporaries in bgen.

Comparisons used to create temporaries for arguments
even if they were already variables or addressable.
Removing the extra ones reduces pressure on regopt.

benchmark                 old ns/op    new ns/op    delta
BenchmarkGobDecode         50787620     49908980   -1.73%
BenchmarkGobEncode         19870190     19473030   -2.00%
BenchmarkGzip            3214321000   3067929000   -4.55%
BenchmarkGunzip           496792800    465828600   -6.23%
BenchmarkJSONEncode       232524800    263864400  +13.48%
BenchmarkJSONDecode       622038400    506600600  -18.56%
BenchmarkMandelbrot200     23937310     45913060  +91.81%
BenchmarkParse             14364450     13997010   -2.56%
BenchmarkRevcomp         6919028000   6480009000   -6.35%
BenchmarkTemplate         594458800    539528200   -9.24%

benchmark                  old MB/s     new MB/s  speedup
BenchmarkGobDecode            15.11        15.38    1.02x
BenchmarkGobEncode            38.63        39.42    1.02x
BenchmarkGzip                  6.04         6.33    1.05x
BenchmarkGunzip               39.06        41.66    1.07x
BenchmarkJSONEncode            8.35         7.35    0.88x
BenchmarkJSONDecode            3.12         3.83    1.23x
BenchmarkParse                 4.03         4.14    1.03x
BenchmarkRevcomp              36.73        39.22    1.07x
BenchmarkTemplate              3.26         3.60    1.10x

R=mtj, daniel.morsing, rsc
CC=golang-dev
http://codereview.appspot.com/6547064