code review 75240043: cmd/6g: fix stack zeroing preamble on amd64p32.

cmd/6g: fix stack zeroing preamble on amd64p32.

It was using a REP STOSQ but putting in CX the number of 32-bit
words to clear.

[remyoudompheng, 2014-03-13 06:12:44]

Hello rsc@golang.org, dave@cheney.net (cc: golang-codereviews@googlegroups.com),

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

[dave_cheney.net, 2014-03-13 06:44:30]

LGTM. This matches the original CL
https://codereview.appspot.com/15540045/diff/190001/src/cmd/6g/ggen.c

This also makes me worry that maybe the rest of that CL is missing as well.

On Thu, Mar 13, 2014 at 5:12 PM,  <remyoudompheng@gmail.com> wrote:
> Reviewers: rsc, dfc,
>
> Message:
> Hello rsc@golang.org, dave@cheney.net (cc:
> golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> cmd/6g: fix stack zeroing preamble on amd64p32.
>
> It was using a REP STOSQ but putting in CX the number of 32-bit
> words to clear.
>
> Please review this at https://codereview.appspot.com/75240043/
>
> Affected files (+4, -4 lines):
>   M src/cmd/6g/ggen.c
>
>
> Index: src/cmd/6g/ggen.c
> ===================================================================
> --- a/src/cmd/6g/ggen.c
> +++ b/src/cmd/6g/ggen.c
> @@ -30,11 +30,11 @@
>         // when it looks for pointers.
>         p = ptxt;
>         if(stkzerosize > 0) {
> -               p = appendpp(p, AMOVQ, D_CONST, 0, D_AX, 0);
> -               p = appendpp(p, AMOVQ, D_CONST, stkzerosize/widthptr, D_CX,
> 0);
> -               p = appendpp(p, ALEAQ, D_SP+D_INDIR, frame-stkzerosize,
> D_DI, 0);
> +               p = appendpp(p, movptr, D_CONST, 0, D_AX, 0);
> +               p = appendpp(p, movptr, D_CONST, stkzerosize/widthptr, D_CX,
> 0);
> +               p = appendpp(p, leaptr, D_SP+D_INDIR, frame-stkzerosize,
> D_DI, 0);
>                 p = appendpp(p, AREP, D_NONE, 0, D_NONE, 0);
> -               appendpp(p, ASTOSQ, D_NONE, 0, D_NONE, 0);
> +               appendpp(p, stosptr, D_NONE, 0, D_NONE, 0);
>         }
>  }
>
>
>

[remyoudompheng, 2014-03-13 07:13:19]

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

cmd/6g: fix stack zeroing preamble on amd64p32.

It was using a REP STOSQ but putting in CX the number of 32-bit
words to clear.

LGTM=dave
R=rsc, dave
CC=golang-codereviews
https://codereview.appspot.com/75240043

[rsc, 2014-03-13 13:36:03]

LGTM

The rest of the CL was applied correctly. The zeroing code was missed
because it had been removed. Now it's back.

[dave_cheney.net, 2014-03-13 13:53:32]

Thanks for confirming Russ, I spent some time going through he commits looking
for other places I may have missed. 

The one outstanding area is reflect, I have a small change to propose which
picks up a few bits that didn't make it into the original merge -- mainly
because nacl wasn't working and I didn't want to mess with the other platforms.


I would appreciate your help reviewing those as this is some deep magic. 

Cheers

Dave

> On 14 Mar 2014, at 0:36, Russ Cox <rsc@golang.org> wrote:
> 
> LGTM
> 
> The rest of the CL was applied correctly. The zeroing code was missed because
it had been removed. Now it's back.
>