code review 86270043: cmd/6g: nacl: zero odd multiple of widthptr correctly

cmd/6g: nacl: zero odd multiple of widthptr correctly

[khr, 2014-04-10 00:07:58]

Hello remyoudompheng@gmail.com (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://khr%40golang.org@code.google.com/p/go/

[iant, 2014-04-10 00:16:05]

LGTM

https://codereview.appspot.com/86270043/diff/1/src/cmd/6g/ggen.c
File src/cmd/6g/ggen.c (right):

https://codereview.appspot.com/86270043/diff/1/src/cmd/6g/ggen.c#newcode80
src/cmd/6g/ggen.c:80: p = appendpp(p, AMOVL, D_AX, 0, D_SP+D_INDIR, frame+lo);
Do we know anything about the alignment of the values we are zeroing?  If they
are 8-byte aligned, then this will push them to be 4-byte aligned causing a
penalty each time the subequent 8-byte writes cross a cache line.  But if we
don't know anything about the alignment then it doesn't matter.

[khr, 2014-04-10 14:59:07]

On 2014/04/10 00:16:05, iant wrote:
> LGTM
> 
> https://codereview.appspot.com/86270043/diff/1/src/cmd/6g/ggen.c
> File src/cmd/6g/ggen.c (right):
> 
> https://codereview.appspot.com/86270043/diff/1/src/cmd/6g/ggen.c#newcode80
> src/cmd/6g/ggen.c:80: p = appendpp(p, AMOVL, D_AX, 0, D_SP+D_INDIR, frame+lo);
> Do we know anything about the alignment of the values we are zeroing?  If they
> are 8-byte aligned, then this will push them to be 4-byte aligned causing a
> penalty each time the subequent 8-byte writes cross a cache line.  But if we
> don't know anything about the alignment then it doesn't matter.

Object sizes are always a multiple of their alignments.  So if the size is an
odd multiple of 4, the alignment is (at most) 4.

I'm guessing that more often than not we'll get unlucky and be 8 byte aligned,
but I'm not too worried about it.  The split line penalty is small and the
chances of hitting a line boundary are small also.

[khr, 2014-04-10 14:59:52]

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

cmd/6g: nacl: zero odd multiple of widthptr correctly

LGTM=iant
R=remyoudompheng, iant
CC=golang-codereviews
https://codereview.appspot.com/86270043

[gobot, 2014-04-10 15:39:30]

This CL appears to have broken the netbsd-amd64-bsiegert builder.
See http://build.golang.org/log/a258b194e5a3da177d58fafda0d47e8dbf7ef73e