code review 6551067: runtime: prepare for 64-bit ints

runtime: prepare for 64-bit ints

This CL makes the runtime understand that the type of
the len or cap of a map, slice, or string is 'int', not 'int32',
and it is also careful to distinguish between function arguments
and results of type 'int' vs type 'int32'.

In the runtime, the new typedefs 'intgo' and 'uintgo' refer
to Go int and uint. The C types int and uint continue to be
unavailable (cause intentional compile errors).

This CL does not change the meaning of int, but it should make
the eventual change of the meaning of int on amd64 a bit
smoother.

Update issue 2188.

[rsc, 2012-09-24 02:49:40]

Hello iant, r (cc: golang-dev@googlegroups.com),

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

[r, 2012-09-24 03:28:40]

LGTM but wait for iant
i like the exposed symmetry break via intgo and uintgo
*

http://codereview.appspot.com/6551067/diff/1002/src/pkg/runtime/runtime.h
File src/pkg/runtime/runtime.h (right):

http://codereview.appspot.com/6551067/diff/1002/src/pkg/runtime/runtime.h#new...
src/pkg/runtime/runtime.h:23: typedef	uint32		uintgo;
might as well do "Go's uint" for parallelism. there are several appearances

[dave_cheney.net, 2012-09-24 03:58:31]

LGTM. I agree with r, intgo and friends grow on you after a while.

http://codereview.appspot.com/6551067/diff/1002/src/pkg/runtime/malloc.h
File src/pkg/runtime/malloc.h (right):

http://codereview.appspot.com/6551067/diff/1002/src/pkg/runtime/malloc.h#newc...
src/pkg/runtime/malloc.h:138: #endif
This is an additional place to update when addressing issue 2142. I think you
should mention this in 2142, or add

Updates issues 2142.

to the description.

[rsc, 2012-09-24 04:08:18]

> This is an additional place to update when addressing issue 2142. I
> think you should mention this in 2142

Done.

[rsc, 2012-09-24 04:09:14]

> might as well do "Go's uint" for parallelism.

Done. Will wait for iant.

[remyoudompheng, 2012-09-24 05:58:12]

http://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/hashmap.c
File src/pkg/runtime/hashmap.c (right):

http://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/hashmap.c#new...
src/pkg/runtime/hashmap.c:731: // makemap(typ *Type, hint uint32) (hmap
*map[any]any);
this seems mismatching

http://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/malloc.goc
File src/pkg/runtime/malloc.goc (right):

http://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/malloc.goc#ne...
src/pkg/runtime/malloc.goc:31: int32 sizeclass, rate;
rate should be intgo

http://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/malloc.goc#ne...
src/pkg/runtime/malloc.goc:229: int32 rate;
intgo

[rsc, 2012-09-24 16:38:39]

>
http://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/hashmap.c#new...
> src/pkg/runtime/hashmap.c:731: // makemap(typ *Type, hint uint32) (hmap
> *map[any]any);
> this seems mismatching

Yeah, dreg comment. This is not called directly from Go. Deleted.

>
http://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/malloc.goc#ne...
> src/pkg/runtime/malloc.goc:31: int32 sizeclass, rate;
> rate should be intgo
>
>
http://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/malloc.goc#ne...
> src/pkg/runtime/malloc.goc:229: int32 rate;
> intgo

Fixed.

Thanks.
Russ

[iant, 2012-09-24 17:17:07]

LGTM

https://codereview.appspot.com/6551067/diff/2019/src/pkg/reflect/value.go
File src/pkg/reflect/value.go (right):

https://codereview.appspot.com/6551067/diff/2019/src/pkg/reflect/value.go#new...
src/pkg/reflect/value.go:2238: func chancap(ch iword) int32
s/int32/int/

https://codereview.appspot.com/6551067/diff/2019/src/pkg/reflect/value.go#new...
src/pkg/reflect/value.go:2240: func chanlen(ch iword) int32
s/int32/int/

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/chan.c
File src/pkg/runtime/chan.c (right):

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/chan.c#newco...
src/pkg/runtime/chan.c:42: uint32	sendx;			// send index
sendx and recvx should logically now become uintgo.

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/chan.c#newco...
src/pkg/runtime/chan.c:104: c->elemsize = elem->size;
Unrelated to this CL, but this assignment could overflow elemsize, which is a
uint16.

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/chan.c#newco...
src/pkg/runtime/chan.c:1096: chosen = (int32)(uintptr)selectgo(&sel);
s/(int32)/(intgo)/

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/chan.c#newco...
src/pkg/runtime/chan.c:1170: //	func chancap(c chan) (cap int32)
s/int32/int/

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/chan.c#newco...
src/pkg/runtime/chan.c:1172: reflect·chancap(Hchan *c, int32 cap)
s/int32/intgo/

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/malloc.goc
File src/pkg/runtime/malloc.goc (right):

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/malloc.goc#n...
src/pkg/runtime/malloc.goc:510: int32 i, nret;
nret should be intgo or uintptr, which will require knockon changes in mfinal.c
and mgc0.c.  Right now the code assumes that the total size of the return
parameters of a finalizer will fit in an int32, which is safe enough in practice
but technically insufficient.  Could be done in a different CL.

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/slice.c
File src/pkg/runtime/slice.c (right):

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/slice.c#newc...
src/pkg/runtime/slice.c:157: // copy(to any, fr any, wid uintptr) int
s/uintptr/int/ ?

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/string.goc
File src/pkg/runtime/string.goc (right):

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/string.goc#n...
src/pkg/runtime/string.goc:52: if((uint32)l <= ms ||
runtime·cas(&runtime·maxstring, ms, (uint32)l))
Does the wrong thing if the string is more than 4GB.  Since print rejects any
string longer than maxstring, I think maxstring has to change from uint32 to
intgo or uintgo.  (I think it's fine for the runtime printf code to refuse to
print strings above a certain size, but not by printing [invalid string].

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/string.goc#n...
src/pkg/runtime/string.goc:182: uint32 i, l;
s/uint32/intgo/

[rsc, 2012-09-24 18:56:42]

Thank you for all the bug fixes. :-)

https://codereview.appspot.com/6551067/diff/2019/src/pkg/reflect/value.go
File src/pkg/reflect/value.go (right):

https://codereview.appspot.com/6551067/diff/2019/src/pkg/reflect/value.go#new...
src/pkg/reflect/value.go:2238: func chancap(ch iword) int32
On 2012/09/24 17:17:07, iant wrote:
> s/int32/int/

Ouch, thanks. Looked for others and didn't find any but maplen (also fixed).

https://codereview.appspot.com/6551067/diff/2019/src/pkg/reflect/value.go#new...
src/pkg/reflect/value.go:2240: func chanlen(ch iword) int32
On 2012/09/24 17:17:07, iant wrote:
> s/int32/int/

Done.

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/chan.c
File src/pkg/runtime/chan.c (right):

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/chan.c#newco...
src/pkg/runtime/chan.c:42: uint32	sendx;			// send index
On 2012/09/24 17:17:07, iant wrote:
> sendx and recvx should logically now become uintgo.

Done.

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/chan.c#newco...
src/pkg/runtime/chan.c:104: c->elemsize = elem->size;
On 2012/09/24 17:17:07, iant wrote:
> Unrelated to this CL, but this assignment could overflow elemsize, which is a
> uint16.

The compiler checks that but I added a runtime throw for it too.

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/chan.c#newco...
src/pkg/runtime/chan.c:1096: chosen = (int32)(uintptr)selectgo(&sel);
On 2012/09/24 17:17:07, iant wrote:
> s/(int32)/(intgo)/

Done.

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/chan.c#newco...
src/pkg/runtime/chan.c:1170: //	func chancap(c chan) (cap int32)
On 2012/09/24 17:17:07, iant wrote:
> s/int32/int/

Done.

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/chan.c#newco...
src/pkg/runtime/chan.c:1172: reflect·chancap(Hchan *c, int32 cap)
On 2012/09/24 17:17:07, iant wrote:
> s/int32/intgo/

Done.

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/hashmap.c
File src/pkg/runtime/hashmap.c (right):

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/hashmap.c#ne...
src/pkg/runtime/hashmap.c:731: // makemap(typ *Type, hint uint32) (hmap
*map[any]any);
On 2012/09/24 05:58:12, remyoudompheng wrote:
> this seems mismatching

Done.

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/malloc.goc
File src/pkg/runtime/malloc.goc (right):

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/malloc.goc#n...
src/pkg/runtime/malloc.goc:31: int32 sizeclass, rate;
On 2012/09/24 05:58:12, remyoudompheng wrote:
> rate should be intgo

Done.

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/malloc.goc#n...
src/pkg/runtime/malloc.goc:229: int32 rate;
On 2012/09/24 05:58:12, remyoudompheng wrote:
> intgo

Done.

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/malloc.goc#n...
src/pkg/runtime/malloc.goc:510: int32 i, nret;
On 2012/09/24 17:17:07, iant wrote:
> nret should be intgo or uintptr, which will require knockon changes in
mfinal.c
> and mgc0.c.  Right now the code assumes that the total size of the return
> parameters of a finalizer will fit in an int32, which is safe enough in
practice
> but technically insufficient.  Could be done in a different CL.

Done.

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/slice.c
File src/pkg/runtime/slice.c (right):

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/slice.c#newc...
src/pkg/runtime/slice.c:157: // copy(to any, fr any, wid uintptr) int
On 2012/09/24 17:17:07, iant wrote:
> s/uintptr/int/ ?

This is the width of a single element in the slice, so it is uintptr.
(The length and cap are in the slice headers but memmove needs
to know how much to scale them by.)

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/string.goc
File src/pkg/runtime/string.goc (right):

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/string.goc#n...
src/pkg/runtime/string.goc:52: if((uint32)l <= ms ||
runtime·cas(&runtime·maxstring, ms, (uint32)l))
Changed to "string too long".

https://codereview.appspot.com/6551067/diff/2019/src/pkg/runtime/string.goc#n...
src/pkg/runtime/string.goc:182: uint32 i, l;
On 2012/09/24 17:17:07, iant wrote:
> s/uint32/intgo/

Done.

[rsc, 2012-09-24 18:58:38]

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

runtime: prepare for 64-bit ints

This CL makes the runtime understand that the type of
the len or cap of a map, slice, or string is 'int', not 'int32',
and it is also careful to distinguish between function arguments
and results of type 'int' vs type 'int32'.

In the runtime, the new typedefs 'intgo' and 'uintgo' refer
to Go int and uint. The C types int and uint continue to be
unavailable (cause intentional compile errors).

This CL does not change the meaning of int, but it should make
the eventual change of the meaning of int on amd64 a bit
smoother.

Update issue 2188.

R=iant, r, dave, remyoudompheng
CC=golang-dev
http://codereview.appspot.com/6551067