diff mbox

Go patch committed: Size of int is now 64 bits on x86_64

Message ID mcra9uush66.fsf@google.com
State New
Headers show

Commit Message

Ian Lance Taylor Nov. 6, 2012, 6:46 p.m. UTC 
This patch to the Go compiler and library changes the size of the Go
type "int" to be the same as the size of a pointer.  This means that on
x86_64 the size of int will be 64 bits.  This matches the new behaviour
of the other Go compiler, and is the intended implementation for the
future Go 1.1 release.  Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu.  Committed to mainline.

Ian

Comments

Ian Lance Taylor Nov. 6, 2012, 8:59 p.m. UTC | #1 
On Tue, Nov 6, 2012 at 10:46 AM, Ian Lance Taylor <iant@google.com> wrote:
> This patch to the Go compiler and library changes the size of the Go
> type "int" to be the same as the size of a pointer.  This means that on
> x86_64 the size of int will be 64 bits.  This matches the new behaviour
> of the other Go compiler, and is the intended implementation for the
> future Go 1.1 release.  Bootstrapped and ran Go testsuite on
> x86_64-unknown-linux-gnu.  Committed to mainline.

By the way, if you have an existing working directory with
--enable-languages=go, make sure to remove your TARGET/libgo directory
before you build after updating to this patch.  This change requires
rebuilding all the object files, but there is no dependency that will
force that to happen.

Ian
H.J. Lu Nov. 7, 2012, 12:32 a.m. UTC | #2 
On Tue, Nov 6, 2012 at 10:46 AM, Ian Lance Taylor <iant@google.com> wrote:
> This patch to the Go compiler and library changes the size of the Go
> type "int" to be the same as the size of a pointer.  This means that on
> x86_64 the size of int will be 64 bits.  This matches the new behaviour
> of the other Go compiler, and is the intended implementation for the
> future Go 1.1 release.  Bootstrapped and ran Go testsuite on
> x86_64-unknown-linux-gnu.  Committed to mainline.
>
> Ian
>

On x32, I saw

spawn -ignore SIGHUP
/export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/gcc/testsuite/go3/../../gccgo
-B/export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/gcc/testsuite/go3/../../
./tmp.go -fno-diagnostics-show-caret
-I/export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/x86_64-unknown-linux-gnu/./libgo
-w -O0 -g -fno-var-tracking-assignments
-L/export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/x86_64-unknown-linux-gnu/./libgo
-L/export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/x86_64-unknown-linux-gnu/./libgo/.libs
-lm -mx32 -o /export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/gcc/testsuite/go3/tmp.x^M
./tmp.go:932:21: error: array index out of bounds^M
./tmp.go:933:23: error: array index out of bounds^M
./tmp.go:934:23: error: array index out of bounds^M
...

Is this expected?
Ian Lance Taylor Nov. 7, 2012, 12:41 a.m. UTC | #3 
On Tue, Nov 6, 2012 at 4:32 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Nov 6, 2012 at 10:46 AM, Ian Lance Taylor <iant@google.com> wrote:
>> This patch to the Go compiler and library changes the size of the Go
>> type "int" to be the same as the size of a pointer.  This means that on
>> x86_64 the size of int will be 64 bits.  This matches the new behaviour
>> of the other Go compiler, and is the intended implementation for the
>> future Go 1.1 release.  Bootstrapped and ran Go testsuite on
>> x86_64-unknown-linux-gnu.  Committed to mainline.
>>
>> Ian
>>
>
> On x32, I saw
>
> spawn -ignore SIGHUP
> /export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/gcc/testsuite/go3/../../gccgo
> -B/export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/gcc/testsuite/go3/../../
> ./tmp.go -fno-diagnostics-show-caret
> -I/export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/x86_64-unknown-linux-gnu/./libgo
> -w -O0 -g -fno-var-tracking-assignments
> -L/export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/x86_64-unknown-linux-gnu/./libgo
> -L/export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/x86_64-unknown-linux-gnu/./libgo/.libs
> -lm -mx32 -o /export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/gcc/testsuite/go3/tmp.x^M
> ./tmp.go:932:21: error: array index out of bounds^M
> ./tmp.go:933:23: error: array index out of bounds^M
> ./tmp.go:934:23: error: array index out of bounds^M
> ...
>
> Is this expected?

I assume this led to a test failure?

The Go library doesn't have proper support for x32 right now.  I'm not
even sure what the runtime.GOARCH value should be.  I would not be
surprised if there are some Go testsuite failures when running in x32
mode.

Ian
H.J. Lu Nov. 7, 2012, 12:51 a.m. UTC | #4 
On Tue, Nov 6, 2012 at 4:41 PM, Ian Lance Taylor <iant@google.com> wrote:
> On Tue, Nov 6, 2012 at 4:32 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, Nov 6, 2012 at 10:46 AM, Ian Lance Taylor <iant@google.com> wrote:
>>> This patch to the Go compiler and library changes the size of the Go
>>> type "int" to be the same as the size of a pointer.  This means that on
>>> x86_64 the size of int will be 64 bits.  This matches the new behaviour
>>> of the other Go compiler, and is the intended implementation for the
>>> future Go 1.1 release.  Bootstrapped and ran Go testsuite on
>>> x86_64-unknown-linux-gnu.  Committed to mainline.
>>>
>>> Ian
>>>
>>
>> On x32, I saw
>>
>> spawn -ignore SIGHUP
>> /export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/gcc/testsuite/go3/../../gccgo
>> -B/export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/gcc/testsuite/go3/../../
>> ./tmp.go -fno-diagnostics-show-caret
>> -I/export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/x86_64-unknown-linux-gnu/./libgo
>> -w -O0 -g -fno-var-tracking-assignments
>> -L/export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/x86_64-unknown-linux-gnu/./libgo
>> -L/export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/x86_64-unknown-linux-gnu/./libgo/.libs
>> -lm -mx32 -o /export/build/gnu/gcc-x32-mx32-native/build-x86_64-linux/gcc/testsuite/go3/tmp.x^M
>> ./tmp.go:932:21: error: array index out of bounds^M
>> ./tmp.go:933:23: error: array index out of bounds^M
>> ./tmp.go:934:23: error: array index out of bounds^M
>> ...
>>
>> Is this expected?
>
> I assume this led to a test failure?

Yes. I got

FAIL: ./tmp.go compilation,  -O0 -g -fno-var-tracking-assignments

> The Go library doesn't have proper support for x32 right now.  I'm not

libgo works OK on x32 since x32 doesn't need much special treatment.
The above is the only Go regression on x32.
Ian Lance Taylor Nov. 7, 2012, 1:01 a.m. UTC | #5 
On Tue, Nov 6, 2012 at 4:51 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
> FAIL: ./tmp.go compilation,  -O0 -g -fno-var-tracking-assignments
>
>> The Go library doesn't have proper support for x32 right now.  I'm not
>
> libgo works OK on x32 since x32 doesn't need much special treatment.
> The above is the only Go regression on x32.

I expect that libgo is setting runtime.GOARCH == "amd64" for x32.
Based on that, the index.go test assumes that values that do not fit
in 32 bits can be stored in an int.  However, the size of int on x32
will be 32 bits.  We need to decide what runtime.GOARCH should be for
x32.

Ian
H.J. Lu Nov. 7, 2012, 5:22 a.m. UTC | #6 
On Tue, Nov 6, 2012 at 5:01 PM, Ian Lance Taylor <iant@google.com> wrote:
> On Tue, Nov 6, 2012 at 4:51 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> FAIL: ./tmp.go compilation,  -O0 -g -fno-var-tracking-assignments
>>
>>> The Go library doesn't have proper support for x32 right now.  I'm not
>>
>> libgo works OK on x32 since x32 doesn't need much special treatment.
>> The above is the only Go regression on x32.
>
> I expect that libgo is setting runtime.GOARCH == "amd64" for x32.
> Based on that, the index.go test assumes that values that do not fit
> in 32 bits can be stored in an int.  However, the size of int on x32
> will be 32 bits.  We need to decide what runtime.GOARCH should be for
> x32.
>

I'd like to avoid to adding a  new  GOARCH for x32.  Can you
check size of intgo instead of GOARCH?

Thanks.
diff mbox

Patch

diff -r 530c277d39c4 go/gogo.cc
--- a/go/gogo.cc	Tue Nov 06 10:26:29 2012 -0800
+++ b/go/gogo.cc	Tue Nov 06 10:43:54 2012 -0800
@@ -23,8 +23,7 @@ 
 
 // Class Gogo.
 
-Gogo::Gogo(Backend* backend, Linemap* linemap, int int_type_size,
-           int pointer_size)
+Gogo::Gogo(Backend* backend, Linemap* linemap, int, int pointer_size)
   : backend_(backend),
     linemap_(linemap),
     package_(NULL),
@@ -83,6 +82,7 @@ 
   this->add_named_type(Type::make_complex_type("complex128", 128,
 					       RUNTIME_TYPE_KIND_COMPLEX128));
 
+  int int_type_size = pointer_size;
   if (int_type_size < 32)
     int_type_size = 32;
   this->add_named_type(Type::make_integer_type("uint", true,
diff -r 530c277d39c4 libgo/runtime/runtime.h
--- a/libgo/runtime/runtime.h	Tue Nov 06 10:26:29 2012 -0800
+++ b/libgo/runtime/runtime.h	Tue Nov 06 10:43:54 2012 -0800
@@ -41,8 +41,8 @@ 
 typedef signed int   intptr __attribute__ ((mode (pointer)));
 typedef unsigned int uintptr __attribute__ ((mode (pointer)));
 
-typedef int		intgo; // Go's int
-typedef unsigned int	uintgo; // Go's uint
+typedef intptr		intgo; // Go's int
+typedef uintptr		uintgo; // Go's uint
 
 /* Defined types.  */