code review 7098049: cmd/gc: allow registerization of temporaries created by...

cmd/gc: allow registerization of temporaries created by inlining.

Names beginning with a dot are ignored by optimizers.

[remyoudompheng, 2013-01-12 07:37:39]

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

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

[remyoudompheng, 2013-01-12 07:39:31]

Unspaghettifies things a bit for issue 4397.

Before:

--- prog list "main" ---
0010 (test.go:7) TEXT    main+0(SB),$24-0
0011 (test.go:8) LEAQ    go.string.""+0(SB),BX
0012 (test.go:8) MOVQ    (BX),SI
0013 (test.go:8) MOVQ    8(BX),DX
0014 (test.go:9) MOVQ    (BX),CX
0015 (test.go:9) MOVQ    8(BX),BP
0016 (test.go:10) MOVB    $0,.anon2+-1(SP)
0017 (test.go:10) JMP     ,20
0018 (test.go:10) MOVQ    $1,AX
0019 (test.go:10) JMP     ,23
0020 (test.go:10) CMPQ    DX,BP
0021 (test.go:10) JEQ     ,18
0022 (test.go:10) MOVQ    $0,AX
0023 (test.go:10) MOVB    AX,.anon2+-1(SP)
0024 (test.go:10) MOVBQZX .anon2+-1(SP),BX
0025 (test.go:10) CMPB    BX,$0
0026 (test.go:10) JEQ     ,35
0027 (test.go:11) LEAQ    go.string."ok"+0(SB),BX
0028 (test.go:11) LEAQ    (SP),BP
0029 (test.go:11) MOVQ    BP,DI
0030 (test.go:11) MOVQ    BX,SI
0031 (test.go:11) MOVSQ   ,
0032 (test.go:11) MOVSQ   ,
0033 (test.go:11) CALL    ,runtime.printstring+0(SB)
0034 (test.go:11) CALL    ,runtime.printnl+0(SB)
0035 (test.go:13) RET     ,

After:

--- prog list "main" ---
0010 (test.go:7) TEXT    main+0(SB),$16-0
0011 (test.go:8) LEAQ    go.string.""+0(SB),BX
0012 (test.go:8) MOVQ    (BX),DI
0013 (test.go:8) MOVQ    8(BX),DX
0014 (test.go:9) MOVQ    (BX),SI
0015 (test.go:9) MOVQ    8(BX),BP
0016 (test.go:10) MOVQ    $0,AX
0017 (test.go:10) JMP     ,20
0018 (test.go:10) MOVQ    $1,AX
0019 (test.go:10) JMP     ,23
0020 (test.go:10) CMPQ    DX,BP
0021 (test.go:10) JEQ     ,18
0022 (test.go:10) MOVQ    $0,AX
0023 (test.go:10) CMPB    AX,$0
0024 (test.go:10) JEQ     ,33
0025 (test.go:11) LEAQ    go.string."ok"+0(SB),BX
0026 (test.go:11) LEAQ    (SP),BP
0027 (test.go:11) MOVQ    BP,DI
0028 (test.go:11) MOVQ    BX,SI
0029 (test.go:11) MOVSQ   ,
0030 (test.go:11) MOVSQ   ,
0031 (test.go:11) CALL    ,runtime.printstring+0(SB)
0032 (test.go:11) CALL    ,runtime.printnl+0(SB)
0033 (test.go:13) RET     ,

In this case the JMP spaghetti is already simplified by the linker afterwards.

[dave_cheney.net, 2013-01-12 07:40:38]

ooh, I think there is an issue relating to this registered.

On Saturday, January 12, 2013, wrote:

> Reviewers: rsc, lvd2, golang-dev_googlegroups.com,
>
> Message:
> Hello rsc@golang.org, lvd@gmail.com, golang-dev@googlegroups.com (cc:
> golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> cmd/gc: allow registerization of temporaries created by inlining.
>
> Names beginning with a dot are ignored by optimizers.
>
> Please review this at
https://codereview.appspot.**com/7098049/<https://codereview.appspot.com/7098...
>
> Affected files:
>   M src/cmd/gc/dcl.c
>   M src/cmd/gc/inl.c
>
>
> Index: src/cmd/gc/dcl.c
> ==============================**==============================**=======
> --- a/src/cmd/gc/dcl.c
> +++ b/src/cmd/gc/dcl.c
> @@ -638,7 +638,7 @@
>
>                 if(n->left == N) {
>                         // give it a name so escape analysis has nodes to
> work with
> -                       snprint(namebuf, sizeof(namebuf), ".anon%d",
> gen++);
> +                       snprint(namebuf, sizeof(namebuf), "~anon%d",
> gen++);
>                         n->left = newname(lookup(namebuf));
>                         n->left->orig = N;  // signal that the original
> was absent
>
> @@ -653,7 +653,7 @@
>                         *nn = *n->left;
>                         n->left = nn;
>
> -                       snprint(namebuf, sizeof(namebuf), ".anon%d",
> gen++);
> +                       snprint(namebuf, sizeof(namebuf), "~anon%d",
> gen++);
>                         n->left->sym = lookup(namebuf);
>                 }
>
> Index: src/cmd/gc/inl.c
> ==============================**==============================**=======
> --- a/src/cmd/gc/inl.c
> +++ b/src/cmd/gc/inl.c
> @@ -699,7 +699,7 @@
>  {
>         Node *n;
>
> -       snprint(namebuf, sizeof(namebuf), ".r%d", i);
> +       snprint(namebuf, sizeof(namebuf), "~r%d", i);
>         n = newname(lookup(namebuf));
>         n->type = t->type;
>         n->class = PAUTO;
>
>
>

[rsc, 2013-01-18 20:20:02]

LGTM

[remyoudompheng, 2013-01-18 21:25:22]

*** Submitted as https://code.google.com/p/go/source/detail?r=3660e51167b8 ***

cmd/gc: allow registerization of temporaries created by inlining.

Names beginning with a dot are ignored by optimizers.

R=rsc, lvd, golang-dev, dave
CC=golang-dev
https://codereview.appspot.com/7098049