code review 14521056: cmd/8g: work around 64-bit bug in 8c for Plan 9

cmd/8g: work around 64-bit bug in 8c for Plan 9

CL 13602043 was an attempt that didn't address the
actual problem (which was detailed in that thread
after the CL was submitted).

https://codereview.appspot.com/13602043/

CL 13456043 was an earlier attempt by Lucio De Re
that fixed the actual problem but changed too much
code according to Russ.

https://codereview.appspot.com/13456043/

This CL works around the bug in the simplest way
possible: by storing part of the problematic
expression in a temporary.

[ality, 2013-10-11 18:07:17]

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

I'd like you to review this change to
https://code.google.com/p/go/

[r, 2013-10-11 18:38:53]

https://codereview.appspot.com/14521056/diff/1002/src/cmd/8g/ggen.c
File src/cmd/8g/ggen.c (right):

https://codereview.appspot.com/14521056/diff/1002/src/cmd/8g/ggen.c#newcode49
src/cmd/8g/ggen.c:49: for(i=0, j=(stkptrsize-stkzerosize)/widthptr*2;
i<stkzerosize; i+=widthptr, j+=2)
can you move the j stuff back out of the loop too, to minimize the delta?

[ality, 2013-10-11 18:53:14]

PTAL

https://codereview.appspot.com/14521056/diff/1002/src/cmd/8g/ggen.c
File src/cmd/8g/ggen.c (right):

https://codereview.appspot.com/14521056/diff/1002/src/cmd/8g/ggen.c#newcode49
src/cmd/8g/ggen.c:49: for(i=0, j=(stkptrsize-stkzerosize)/widthptr*2;
i<stkzerosize; i+=widthptr, j+=2)
On 2013/10/11 18:38:53, r wrote:
> can you move the j stuff back out of the loop too, to minimize the delta?

Done.

[r, 2013-10-11 21:04:58]

LGTM

[lucio, 2013-10-12 04:27:01]

So now the code looks like this:

		off = frame-stkzerosize;
 		j = (stkptrsize - stkzerosize)/widthptr * 2;
 		for(i=0; i<stkzerosize; i+=widthptr) {
 			if(bvget(bv, j) || bvget(bv, j+1))
				p = appendp(p, AMOVL, D_CONST, 0, D_SP+D_INDIR, off+i);
 			j += 2;
 		}

and my earlier proposal was:

		i = frame - stkzerosize;
		j = (stkptrsize-stkzerosize)/widthptr * 2;
		while (i < frame) {
 			if(bvget(bv, j) || bvget(bv, j+1))
				p = appendp(p, AMOVL, D_CONST, 0, D_SP+D_INDIR, i);
			i += widthptr;
 			j += 2;
 		}

Please can we at least agree to drop the comment about reverting this later?

Lucio.

On 10/11/13, ality@pbrane.org <ality@pbrane.org> wrote:
> PTAL
>
>
> https://codereview.appspot.com/14521056/diff/1002/src/cmd/8g/ggen.c
> File src/cmd/8g/ggen.c (right):
>
> https://codereview.appspot.com/14521056/diff/1002/src/cmd/8g/ggen.c#newcode49
> src/cmd/8g/ggen.c:49: for(i=0, j=(stkptrsize-stkzerosize)/widthptr*2;
> i<stkzerosize; i+=widthptr, j+=2)
> On 2013/10/11 18:38:53, r wrote:
>> can you move the j stuff back out of the loop too, to minimize the
> delta?
>
> Done.
>
> https://codereview.appspot.com/14521056/
>

[r, 2013-10-12 17:34:40]

Sure, that's fine.

[ality, 2013-12-09 22:13:35]

This CL is no longer relevant due to the new linker changes.

Abandoning.

  Anthony

[ality, 2013-12-09 22:14:00]

*** Abandoned ***