code review 6493099: cmd/6g, cmd/8g: eliminate extra agen for nil comparisons.

cmd/6g, cmd/8g: eliminate extra agen for nil comparisons.

Removes an extra LEAL/LEAQ instructions there and usually saves
a useless temporary in the idiom
    if err := foo(); err != nil {...}

Generated code is also less involved:
    MOVQ err+n(SP), AX
    CMPQ AX, $0
(potentially CMPQ n(SP), $0) instead of
    LEAQ err+n(SP), AX
    CMPQ (AX), $0

Update issue 1914.

[remyoudompheng, 2012-09-09 07:39:45]

Hello rsc@golang.org, golang-dev@googlegroups.com (cc:
golang-dev@googlegroups.com, remy@archlinux.org),

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

[DMorsing, 2012-09-09 07:44:09]

I took a look at this section a couple of days ago, but I couldn't find a test
program that would trigger this path. Have you found one?

[remyoudompheng, 2012-09-09 07:48:52]

The  most impressive reduction in stack size is probably

Before:
src/pkg/regexp/syntax/parse.go:1486) TEXT    (*parser).parseClass+0(SB),$328-56

After:
src/pkg/regexp/syntax/parse.go:1486) TEXT    (*parser).parseClass+0(SB),$232-56

[DMorsing, 2012-09-09 08:39:29]

I need to read code more carefully. After a second pass, the code path looks
downright trivial to me.

I might join you on the task of getting rid of wayward agen() calls. Do you have
anything in the pipeline that you havn't posted CLs for?

[remyoudompheng, 2012-09-09 08:52:03]

No pending patches. I have vague projects:
* wondering whether something could be done for issue 2734 (maybe reusing
scratch space for variadic calls)
* extending componentgen to handle structs with a few small fields so they get
registering

But it doesn't seem as straightforward as the other patches. Maybe something
useful could be to eliminate usage of stack in things like
float64fromBits/float64bits. These do not use stack by themselves because their
argument is already addressable, but when it is inlined, the argument is dumped
to the stack even if it was registerized (see generated code for math.Inf or
math.Nan or math.Copysign: these should not be able to cause a stack split).

[nigeltao, 2012-09-09 09:35:38]

Please replace "Part of issue 1914" in the CL description to "Update issue
1914".

http://code.google.com/p/support/wiki/IssueTracker#Integration_with_version_c...

[rsc, 2012-09-11 02:30:45]

LGTM

http://codereview.appspot.com/6493099/diff/4002/src/cmd/6g/cgen.c
File src/cmd/6g/cgen.c (right):

http://codereview.appspot.com/6493099/diff/4002/src/cmd/6g/cgen.c#newcode986
src/cmd/6g/cgen.c:986: yyerror("illegal array comparison");
s/array/slice/ while you are here. thanks.

http://codereview.appspot.com/6493099/diff/4002/src/cmd/6g/cgen.c#newcode991
src/cmd/6g/cgen.c:991: n1.xoffset += Array_array;
also, in a separate CL, if you want to rename these constants to say Slice_
instead of Array_ that'd probably be helpful to future contributors.

it's hard to believe but it took a while before we realized slices and arrays
were different things.

http://codereview.appspot.com/6493099/diff/4002/src/cmd/8g/cgen.c
File src/cmd/8g/cgen.c (right):

http://codereview.appspot.com/6493099/diff/4002/src/cmd/8g/cgen.c#newcode972
src/cmd/8g/cgen.c:972: yyerror("illegal array comparison");
s/array/slice/ while you are here. thanks.

[rsc, 2012-09-11 02:30:51]

LGTM

[rsc, 2012-09-11 02:31:24]

Sigh. The send mail box got unchecked in my first LGTM. Please see my comments
on the rietveld page.

[remyoudompheng, 2012-09-11 05:58:56]

On 2012/09/09 09:35:38, nigeltao wrote:
> Please replace "Part of issue 1914" in the CL description to "Update issue
> 1914".

Done.

[remyoudompheng, 2012-09-11 05:59:09]

Hello daniel.morsing@gmail.com, nigeltao@golang.org, rsc@golang.org (cc:
golang-dev@googlegroups.com, remy@archlinux.org),

Please take another look.

[remyoudompheng, 2012-09-11 05:59:23]

Submitting...

http://codereview.appspot.com/6493099/diff/4002/src/cmd/6g/cgen.c
File src/cmd/6g/cgen.c (right):

http://codereview.appspot.com/6493099/diff/4002/src/cmd/6g/cgen.c#newcode986
src/cmd/6g/cgen.c:986: yyerror("illegal array comparison");
On 2012/09/11 02:30:45, rsc wrote:
> s/array/slice/ while you are here. thanks.

Done.

http://codereview.appspot.com/6493099/diff/4002/src/cmd/8g/cgen.c
File src/cmd/8g/cgen.c (right):

http://codereview.appspot.com/6493099/diff/4002/src/cmd/8g/cgen.c#newcode972
src/cmd/8g/cgen.c:972: yyerror("illegal array comparison");
On 2012/09/11 02:30:45, rsc wrote:
> s/array/slice/ while you are here. thanks.

Done.

[remyoudompheng, 2012-09-11 06:08:44]

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

cmd/6g, cmd/8g: eliminate extra agen for nil comparisons.

Removes an extra LEAL/LEAQ instructions there and usually saves
a useless temporary in the idiom
    if err := foo(); err != nil {...}

Generated code is also less involved:
    MOVQ err+n(SP), AX
    CMPQ AX, $0
(potentially CMPQ n(SP), $0) instead of
    LEAQ err+n(SP), AX
    CMPQ (AX), $0

Update issue 1914.

R=daniel.morsing, nigeltao, rsc
CC=golang-dev, remy
http://codereview.appspot.com/6493099