code review 6501110: cmd/6g, cmd/8g: do not LEA[LQ] interfaces when calling ...

cmd/6g, cmd/8g: do not LEA[LQ] interfaces when calling methods.

It is enough to load directly the data word and the itab word
from memory, so we save a LEA instruction for each method call,
and allow elimination of some extra temporaries.

Update issue 1914.

[remyoudompheng, 2012-09-09 06:52:49]

Hello 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 06:58:00]

Could you post a section of assembly code before and after this change?

[remyoudompheng, 2012-09-09 07:15:42]

Here is an example where the change leads to elimination of a temporary:
io.WriteString
See how the sw variable got eliminated. Two LEAQ remain, they read function
return values and make the code unnecessarily complex, but they will be
eliminated by the change "make igen a no-op if node is already a OINDREG".

Before

.... prepare assertion w.(stringWriter) ...
0010 (io.go:248) CALL    ,runtime.assertI2I2+0(SB)
0011 (io.go:248) MOVQ    s+16(FP),DX
0012 (io.go:248) MOVL    s+24(FP),CX
0013 (io.go:248) LEAQ    24(SP),BX
0014 (io.go:248) MOVQ    (BX),BP
0015 (io.go:248) MOVQ    BP,sw+-16(SP)
0016 (io.go:248) MOVQ    8(BX),BP
0017 (io.go:248) MOVQ    BP,sw+-8(SP)
0018 (io.go:248) MOVBQZX 40(SP),BX
0019 (io.go:248) CMPB    BX,$0
0020 (io.go:248) JEQ     ,38
0021 (io.go:249) LEAQ    8(SP),BX
0022 (io.go:249) MOVQ    DX,(BX)
0023 (io.go:249) MOVL    CX,8(BX)
0024 (io.go:249) LEAQ    sw+-16(SP),BX
0025 (io.go:249) MOVQ    8(BX),BP
0026 (io.go:249) MOVQ    BP,(SP)
0027 (io.go:249) MOVQ    (BX),BX
0028 (io.go:249) MOVQ    32(BX),BX
0029 (io.go:249) CALL    ,BX
0030 (io.go:249) MOVL    24(SP),DX
0031 (io.go:249) LEAQ    32(SP),BX
0032 (io.go:249) MOVQ    (BX),CX
0033 (io.go:249) MOVQ    8(BX),AX
0034 (io.go:249) MOVL    DX,n+32(FP)
0035 (io.go:249) MOVQ    CX,err+40(FP)
0036 (io.go:249) MOVQ    AX,err+48(FP)
0037 (io.go:249) RET     ,
0038 case where the assertion was false

After:

0010 (io.go:248) CALL    ,runtime.assertI2I2+0(SB)
0011 (io.go:248) MOVQ    s+16(FP),DI
0012 (io.go:248) MOVL    s+24(FP),SI
0013 (io.go:248) LEAQ    24(SP),BX
0014 (io.go:248) MOVQ    (BX),DX
0015 (io.go:248) MOVQ    8(BX),CX
0016 (io.go:248) MOVBQZX 40(SP),BX
0017 (io.go:248) CMPB    BX,$0
0018 (io.go:248) JEQ     ,33
0019 (io.go:249) LEAQ    8(SP),BX
0020 (io.go:249) MOVQ    DI,(BX)
0021 (io.go:249) MOVL    SI,8(BX)
0022 (io.go:249) MOVQ    CX,(SP)
0023 (io.go:249) MOVQ    32(DX),BX
0024 (io.go:249) CALL    ,BX
0025 (io.go:249) MOVL    24(SP),DX
0026 (io.go:249) LEAQ    32(SP),BX
0027 (io.go:249) MOVQ    (BX),CX
0028 (io.go:249) MOVQ    8(BX),AX
0029 (io.go:249) MOVL    DX,n+32(FP)
0030 (io.go:249) MOVQ    CX,err+40(FP)
0031 (io.go:249) MOVQ    AX,err+48(FP)
0032 (io.go:249) RET     ,
0033 assertion is false

[rsc, 2012-09-11 02:25:57]

LGTM

http://codereview.appspot.com/6501110/diff/3/src/cmd/6g/ggen.c
File src/cmd/6g/ggen.c (right):

http://codereview.appspot.com/6501110/diff/3/src/cmd/6g/ggen.c#newcode130
src/cmd/6g/ggen.c:130: igen(i, &nodr, res);		// REG = &inter
Can you introduce a new variable nodi and use it here and below? I got confused
reading this about nodr being smashed by the REG = 32+... line, but it isn't
smashed: it got regalloc'ed midway. Having a separate variable will avoid that
confusion the next time someone reads it.

Thanks.

http://codereview.appspot.com/6501110/diff/3/src/cmd/8g/ggen.c
File src/cmd/8g/ggen.c (right):

http://codereview.appspot.com/6501110/diff/3/src/cmd/8g/ggen.c#newcode167
src/cmd/8g/ggen.c:167: igen(i, &nodr, res);		// REG = &inter
Same.

[remyoudompheng, 2012-09-11 06:42:20]

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

Please take another look.

[remyoudompheng, 2012-09-11 06:45:06]

Submitting.

http://codereview.appspot.com/6501110/diff/3/src/cmd/6g/ggen.c
File src/cmd/6g/ggen.c (right):

http://codereview.appspot.com/6501110/diff/3/src/cmd/6g/ggen.c#newcode130
src/cmd/6g/ggen.c:130: igen(i, &nodr, res);		// REG = &inter
On 2012/09/11 02:25:58, rsc wrote:
> Can you introduce a new variable nodi and use it here and below? I got
confused
> reading this about nodr being smashed by the REG = 32+... line, but it isn't
> smashed: it got regalloc'ed midway. Having a separate variable will avoid that
> confusion the next time someone reads it.
> 
> Thanks.

Done.

http://codereview.appspot.com/6501110/diff/3/src/cmd/8g/ggen.c
File src/cmd/8g/ggen.c (right):

http://codereview.appspot.com/6501110/diff/3/src/cmd/8g/ggen.c#newcode167
src/cmd/8g/ggen.c:167: igen(i, &nodr, res);		// REG = &inter
On 2012/09/11 02:25:58, rsc wrote:
> Same.

Done.

[remyoudompheng, 2012-09-11 06:45:28]

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

cmd/6g, cmd/8g: do not LEA[LQ] interfaces when calling methods.

It is enough to load directly the data word and the itab word
from memory, so we save a LEA instruction for each method call,
and allow elimination of some extra temporaries.

Update issue 1914.

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