code review 6620050: runtime: update comment for the "extern register" varia...

runtime: update comment for the "extern register" variables g and m.

[nigeltao, 2012-10-05 01:59:56]

Hello rsc@golang.org (cc: dave@cheney.net, golang-dev@googlegroups.com),

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

[dave_cheney.net, 2012-10-05 02:09:48]

This is correct, from 5g/gsubr.c ~ 308

   308	static	int	resvd[] =
   309	{
   310		9,	// reserved for m
   311		10,	// reserved for g
   312	};

On Fri, Oct 5, 2012 at 11:59 AM,  <nigeltao@golang.org> wrote:
> Reviewers: rsc,
>
> Message:
> Hello rsc@golang.org (cc: dave@cheney.net, golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> runtime: update comment for the "extern register" variables g and m.
>
> The locations were determined by disassembling (in gdb) the binaries
> generated by 6g/8g. I don't know where in the C compiler they are
> programatically determined.
>
>
> on amd64, disassembling 6c code for "if(g)" and "if(m)" gives:
>         mov    %fs:0xfffffffffffffff0,%rax
>         cmp    $0x0,%rax
> and
>         mov    %fs:0xfffffffffffffff8,%rax
>         cmp    $0x0,%rax
>
>
> on 386, disassembling 8c code for "if(g)" and "if(m)" gives:
>         mov    %gs:0x0,%eax
>         mov    -0x8(%eax),%eax
>         cmp    $0x0,%eax
> and
>         mov    %gs:0x0,%eax
>         mov    -0x4(%eax),%eax
>         cmp    $0x0,%eax
>
> I don't have an ARM system around to verify R10/R9. Dave Cheney, are
> you able to verify this?
>
> Please review this at http://codereview.appspot.com/6620050/
>
> Affected files:
>   M src/pkg/runtime/runtime.h
>
>
> Index: src/pkg/runtime/runtime.h
> ===================================================================
> --- a/src/pkg/runtime/runtime.h
> +++ b/src/pkg/runtime/runtime.h
> @@ -84,17 +84,18 @@
>  /*
>   * per-cpu declaration.
>   * "extern register" is a special storage class implemented by 6c, 8c, etc.
> - * on machines with lots of registers, it allocates a register that will
> not be
> - * used in generated code.  on the x86, it allocates a slot indexed by a
> - * segment register.
>   *
> - * amd64: allocated downwards from R15
> - * x86: allocated upwards from 0(GS)
> - * arm: allocated downwards from R10
> + * On amd64 and x86, it allocates a slot indexed by a segment register.
> + * On arm, it allocates a register that will not be used in generated code.
>   *
> - * every C file linked into a Go program must include runtime.h
> - * so that the C compiler knows to avoid other uses of these registers.
> - * the Go compilers know to avoid them.
> + * On amd64: g is -16(FS) and m is -8(FS).
> + * On x86:   g is  -8(GS) and m is -4(GS).
> + * On arm:   g is     R10 and m is    R9.
> + * TODO: verify arm registers.
> + *
> + * Every C file linked into a Go program must include runtime.h so that the
> + * C compiler (6c, 8c, etc.) knows to avoid other uses of these registers.
> + * The Go compiler (6g, 8g, etc.) knows to avoid them.
>   */
>  extern register        G*      g;
>  extern register        M*      m;
>
>

[minux1, 2012-10-05 04:43:54]

i can't use rieltveld atm.

On Oct 5, 2012 9:59 AM, <nigeltao@golang.org> wrote:
> - * the Go compilers know to avoid them.
> + * On amd64: g is -16(FS) and m is -8(FS).
> + * On x86:   g is  -8(GS) and m is -4(GS).
> + * On arm:   g is     R10 and m is    R9.
i suggest we don't specify these details here.
because iirc, different OS use different segment register
and/or offset (the linker can rewrite these offset).

for details, please refer to zasmhdr array in src/cmd/dist/buildruntime.c
or src/pkg/runtime/zasm_$GOOS_$GOARCH.h.

[nigeltao, 2012-10-05 05:17:53]

On 5 October 2012 14:43, minux <minux.ma@gmail.com> wrote:
> i suggest we don't specify these details here.
> because iirc, different OS use different segment register
> and/or offset (the linker can rewrite these offset).

Oh, I see. Still, the previous comment "amd64: allocated downwards
from R15" was misleading. I've updated the change description to just
be

runtime: update comment for the "extern register" variables g and m.

and the comment now says

/*
 * Per-CPU declaration.
 *
 * "extern register" is a special storage class implemented by 6c, 8c, etc.
 * It may be an actual dedicated register such as R10, or a slot indexed by
 * a register such as 4(GS). The register and index used depends on the OS
 * and architecture: see zasmhdr in src/cmd/dist/buildruntime.c.
 *
 * Every C file linked into a Go program must include runtime.h so that the
 * C compiler (6c, 8c, etc.) knows to avoid other uses of these dedicated
 * registers. The Go compiler (6g, 8g, etc.) knows to avoid them.
 */

WDYT?

[minux1, 2012-10-05 05:39:38]

On Oct 5, 2012 1:17 PM, "Nigel Tao" <nigeltao@golang.org> wrote:
> and the comment now says
> [snip]
>  * It may be an actual dedicated register such as R10, or a slot indexed
by
>  * a register such as 4(GS).
the details are still more subtle than this, there could be
two indirections involved (and ld's possible rewrite just
complicate it more).
but i can't find better ways to explain this, so lgtm,
let's wait for rsc.

i suggest s/a register/a segment register/.

[ality, 2012-10-05 17:54:32]

It's a bit more nuanced.

For x86 and amd64, extern registers are allocated
by the compiler upwards from 0(GS). See:

    cc/dcl.c:/^xdecl
    6c/txt.c:/^exreg
    6c/txt.c:/^naddr

The linker transforms these generic references
into OS-specific references. See:

    6l/obj.c:/tlsoffset
    6l/pass.c:/^patch

Your new comment is wrong for Darwin, Windows, and Plan 9
(essentially everything that isn't ELF). The assembly for
64-bit Darwin would look something like:

if(g)
    mov  %gs:0x8a0,%rax
	cmp  %0x0,%rax

if(m)
    mov  %gs:0x8a8,%rax
	cmp  %0x0,%rax

Hope that helps.

Cheers,
  Anthony

[rsc, 2012-10-06 21:59:41]

https://codereview.appspot.com/6620050/diff/9002/src/pkg/runtime/runtime.h
File src/pkg/runtime/runtime.h (right):

https://codereview.appspot.com/6620050/diff/9002/src/pkg/runtime/runtime.h#ne...
src/pkg/runtime/runtime.h:88: * It may be an actual dedicated register such as
R10, or a slot indexed by
 * On the ARM, it is an actual register; elsewhere it is a slot in thread-local
storage
 * indexed by a segment register.
 *
 * Every C file...

Even src/cmd/dist/buildruntime.c is not the full story. 0(GS) and 4(GS)/8(GS)
are really even themselves pseudo-locations turned into real things by the
linker. But let's not get into that here.

https://codereview.appspot.com/6620050/diff/9002/src/pkg/runtime/runtime.h#ne...
src/pkg/runtime/runtime.h:89: * a register such as 4(GS). The register and index
used depends on the OS
s/register/segment register/

[nigeltao, 2012-10-12 07:23:19]

PTAL.

[minux1, 2012-10-12 08:06:44]

LGTM.

[nigeltao, 2012-10-19 00:09:27]

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

runtime: update comment for the "extern register" variables g and m.

R=rsc, minux.ma, ality
CC=dave, golang-dev
http://codereview.appspot.com/6620050