Issue 7536046: code review 7536046: runtime: use 64-bit negative error code on 64-bit machines

Issue 7536046: code review 7536046: runtime: use 64-bit negative error code on 64-bit machines (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
11 years, 1 month ago by rsc

Modified:
11 years, 1 month ago

Reviewers:

CC:
golang-dev, bradfitz

Visibility:
Public.

Description

runtime: use 64-bit negative error code on 64-bit machines NEGL does a negation of the bottom 32 bits and then zero-extends to 64 bits, resulting in a negative 32-bit number but a positive 64-bit number. NEGQ does a full 64-bit negation, so that the result is negative both as a 32-bit and as a 64-bit number. This doesn't matter for the functions that are declared to return int32. It only matters for the ones that return int64 or void* [sic]. This will fix the current incorrect error in the OpenBSD/amd64 build. The build will still be broken, but it won't report a bogus error.

Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+12 lines, -12 lines)			Patch
src/pkg/runtime/sys_darwin_amd64.s	View	1	5 chunks	+5 lines, -5 lines	0 comments	Download
src/pkg/runtime/sys_freebsd_amd64.s	View	1	1 chunk	+1 line, -1 line	0 comments	Download
src/pkg/runtime/sys_netbsd_amd64.s	View	1	3 chunks	+3 lines, -3 lines	0 comments	Download
src/pkg/runtime/sys_openbsd_amd64.s	View	1	3 chunks	+3 lines, -3 lines	0 comments	Download

Messages

Total messages: 3

Expand All Messages | Collapse All Messages

rsc

Hello golang-dev@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg/

11 years, 1 month ago (2013-03-14 21:38:31 UTC) #1

bradfitz

LGTM On Thu, Mar 14, 2013 at 2:38 PM, <rsc@golang.org> wrote: > Reviewers: golang-dev1, > ...

11 years, 1 month ago (2013-03-14 21:55:10 UTC) #2

LGTM

On Thu, Mar 14, 2013 at 2:38 PM, <rsc@golang.org> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> runtime: use 64-bit negative error code on 64-bit machines
>
> NEGL does a negation of the bottom 32 bits and then zero-extends to 64
> bits,
> resulting in a negative 32-bit number but a positive 64-bit number.
>
> NEGQ does a full 64-bit negation, so that the result is negative both as
> a 32-bit and as a 64-bit number.
>
> This doesn't matter for the functions that are declared to return int32.
> It only matters for the ones that return int64 or void* [sic].
>
> This will fix the current incorrect error in the OpenBSD/amd64 build.
> The build will still be broken, but it won't report a bogus error.
>
> Please review this at
https://codereview.appspot.**com/7536046/<https://codereview.appspot.com/7536...
>
> Affected files:
>   M src/pkg/runtime/sys_darwin_**amd64.s
>   M src/pkg/runtime/sys_freebsd_**amd64.s
>   M src/pkg/runtime/sys_netbsd_**amd64.s
>   M src/pkg/runtime/sys_openbsd_**amd64.s
>
>
> Index: src/pkg/runtime/sys_darwin_**amd64.s
> ==============================**==============================**=======
> --- a/src/pkg/runtime/sys_darwin_**amd64.s
> +++ b/src/pkg/runtime/sys_darwin_**amd64.s
> @@ -289,7 +289,7 @@
>         MOVQ    $(0x2000000+360), AX    // bsdthread_create
>         SYSCALL
>         JCC 3(PC)
> -       NEGL    AX
> +       NEGQ    AX
>         RET
>         MOVL    $0, AX
>         RET
> @@ -342,7 +342,7 @@
>         MOVQ    $(0x2000000+366), AX    // bsdthread_register
>         SYSCALL
>         JCC 3(PC)
> -       NEGL    AX
> +       NEGQ    AX
>         RET
>         MOVL    $0, AX
>         RET
> @@ -435,7 +435,7 @@
>         MOVL    $(0x2000000+202), AX    // syscall entry
>         SYSCALL
>         JCC 3(PC)
> -       NEGL    AX
> +       NEGQ    AX
>         RET
>         MOVL    $0, AX
>         RET
> @@ -448,7 +448,7 @@
>         MOVL    $(0x2000000+362), AX
>         SYSCALL
>         JCC     2(PC)
> -       NEGL    AX
> +       NEGQ    AX
>         RET
>
>  // int32 runtime·kevent(int kq, Kevent *changelist, int nchanges, Kevent
> *eventlist, int nevents, Timespec *timeout);
> @@ -462,7 +462,7 @@
>         MOVL    $(0x2000000+363), AX
>         SYSCALL
>         JCC     2(PC)
> -       NEGL    AX
> +       NEGQ    AX
>         RET
>
>  // void runtime·closeonexec(int32 fd);
> Index: src/pkg/runtime/sys_freebsd_**amd64.s
> ==============================**==============================**=======
> --- a/src/pkg/runtime/sys_freebsd_**amd64.s
> +++ b/src/pkg/runtime/sys_freebsd_**amd64.s
> @@ -261,7 +261,7 @@
>         MOVQ    $202, AX                // sys___sysctl
>         SYSCALL
>         JCC 3(PC)
> -       NEGL    AX
> +       NEGQ    AX
>         RET
>         MOVL    $0, AX
>         RET
> Index: src/pkg/runtime/sys_netbsd_**amd64.s
> ==============================**==============================**=======
> --- a/src/pkg/runtime/sys_netbsd_**amd64.s
> +++ b/src/pkg/runtime/sys_netbsd_**amd64.s
> @@ -16,7 +16,7 @@
>         MOVL    $309, AX                // sys__lwp_create
>         SYSCALL
>         JCC     2(PC)
> -       NEGL    AX
> +       NEGQ    AX
>         RET
>
>  TEXT runtime·lwp_tramp(SB),7,$0
> @@ -254,7 +254,7 @@
>         MOVL    $197, AX                // sys_mmap
>         SYSCALL
>         JCC     2(PC)
> -       NEGL    AX
> +       NEGQ    AX
>         ADDQ    $16, SP
>         RET
>
> @@ -306,7 +306,7 @@
>         MOVQ    $202, AX                // sys___sysctl
>         SYSCALL
>         JCC 3(PC)
> -       NEGL    AX
> +       NEGQ    AX
>         RET
>         MOVL    $0, AX
>         RET
> Index: src/pkg/runtime/sys_openbsd_**amd64.s
> ==============================**==============================**=======
> --- a/src/pkg/runtime/sys_openbsd_**amd64.s
> +++ b/src/pkg/runtime/sys_openbsd_**amd64.s
> @@ -23,7 +23,7 @@
>
>         // Return if tfork syscall failed.
>         JCC     3(PC)
> -       NEGL    AX
> +       NEGQ    AX
>         RET
>
>         // In parent, return.
> @@ -243,7 +243,7 @@
>         MOVL    $197, AX
>         SYSCALL
>         JCC     2(PC)
> -       NEGL    AX
> +       NEGQ    AX
>         ADDQ    $16, SP
>         RET
>
> @@ -294,7 +294,7 @@
>         MOVQ    $202, AX                // sys___sysctl
>         SYSCALL
>         JCC     3(PC)
> -       NEGL    AX
> +       NEGQ    AX
>         RET
>         MOVL    $0, AX
>         RET
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

rsc

11 years, 1 month ago (2013-03-15 03:42:16 UTC) #3

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

runtime: use 64-bit negative error code on 64-bit machines

NEGL does a negation of the bottom 32 bits and then zero-extends to 64 bits,
resulting in a negative 32-bit number but a positive 64-bit number.

NEGQ does a full 64-bit negation, so that the result is negative both as
a 32-bit and as a 64-bit number.

This doesn't matter for the functions that are declared to return int32.
It only matters for the ones that return int64 or void* [sic].

This will fix the current incorrect error in the OpenBSD/amd64 build.
The build will still be broken, but it won't report a bogus error.

R=golang-dev, bradfitz
CC=golang-dev
https://codereview.appspot.com/7536046

Expand All Messages | Collapse All Messages