code review 101510049: liblink, runtime: preliminary support for plan9/amd64

liblink, runtime: preliminary support for plan9/amd64

A TLS slot is reserved by _rt0_.*_plan9 as an automatic and
its address (which is static on Plan 9) is saved in the
global _privates symbol. The startup linkage now is exactly
like that from Plan 9 libc, and the way we access g is
exactly as if we'd have used privalloc(2).

Aside from making the code more standard, this change
drastically simplifies it, both for 386 and for amd64, and
makes the Plan 9 code in liblink common for both 386 and
amd64.

The amd64 runtime code was cleared of nxm assumptions, and
now runs on the standard Plan 9 kernel.

Note handling fixes will follow in a separate CL.

[aram, 2014-06-30 12:33:43]

Hello golang-codereviews@googlegroups.com (cc: 0intro, ality, jas, minux1,
mischief@9.offblast.org, rsc),

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

[rsc, 2014-06-30 15:46:42]

If this were a completely new port you'd need a builder running before we
started reviewing changes to support it. See golang.org/wiki/PortingPolicy.

I am inclined to treat this as a completely new port because (1) Plan 9 on amd64
has never worked, and (2) the porting policy was motivated exactly by the Plan 9
386 port dragging on forever.

How close are you to having a builder running?

Thanks.

[aram, 2014-06-30 16:17:07]

David says he can put a machine up by the end of July. I don't have any amd64
machine to spare that's quiet enough to keep on at all times around me. If
anyone else can donate a machine, we can start a builder today.

I did this because I wanted to move the arm64 compiler work (which is being done
on Plan 9, btw) on my amd64 cpu servers instead of having to keep 386 cpu
servers around just because of Go. It only took me two hours to do this CL; Go
plan9/amd64 works well enough now that I switched to it already. Needless to
say, there are many more things to be done. Almost all of them affect plan9/386
as well, because that port has a ton of problems, even though it passes the test
suite (or what's left of it after disabling most "hard" tests). 

Software gets better when it's used, and when the people that use it can fix it.
I use both Go and plan9/amd64 every day. It would appear this was not true for
the people previously in charge of plan9/amd64. I hope that I proved already
that I can fix and maintain it by doing the Solaris port which has been of
excellent quality and not a hackjob without maintainer like the DragonflyBSD
port. The Plan 9 port is also full of hacks and misguided code. While improving
the amd64 port, all that junk will go away, and the 386 port will benefit too. I
have a bunch of such fixes in the pipeline.

I am very sorry I don't have a machine to spare.

[rsc, 2014-06-30 16:22:13]

I am also a little confused by "The amd64 runtime code was cleared of nxm
assumptions, and now runs on the standard Plan 9 kernel." Is the amd64
kernel in the standard distribution? I looked through /sys/src/9 and can't
find any kernels with objtype=amd64 in their mkfiles.

[aram, 2014-06-30 16:33:42]

> I am also a little confused by "The amd64 runtime code was cleared of nxm
> assumptions, and now runs on the standard Plan 9 kernel." Is the amd64
> kernel in the standard distribution? I looked through /sys/src/9 and can't
> find any kernels with objtype=amd64 in their mkfiles.

Sorry for the confusing phrasing, I'll think of something better. There are (at
least) 5 amd64 kernels in various places, and neither is in the Bell Labs
distribution. There's the original nix kernel, there's Erik's nix in 9atom,
there's some stuff from Nemo that's derived from nix, there's nxm, which is also
nix-derived, and there's the pc64 kernel in 9front. The nix kernels are
divergent from the original kernel. They sit in /sys/src/nix instead of
/sys/src/9. Pc64 is a straight port of the "standard" /sys/src/9/pc kernel.

Pc64 uses 4k pages and exposes the same system calls and interfaces like pc. The
nix kernels come with 2MB-1GB pages, new system calls and new ways of doing
things (like the use of segment registers). This port targets both pc64 and
nix-derived kernels. It doesn't use the crazy stuff from the nix kernels.

[rsc, 2014-06-30 17:23:22]

And where is pc64?


On Mon, Jun 30, 2014 at 12:33 PM, <aram@mgk.ro> wrote:

> I am also a little confused by "The amd64 runtime code was cleared of
>>
> nxm
>
>> assumptions, and now runs on the standard Plan 9 kernel." Is the amd64
>> kernel in the standard distribution? I looked through /sys/src/9 and
>>
> can't
>
>> find any kernels with objtype=amd64 in their mkfiles.
>>
>
> Sorry for the confusing phrasing, I'll think of something better. There
> are (at least) 5 amd64 kernels in various places, and neither is in the
> Bell Labs distribution. There's the original nix kernel, there's Erik's
> nix in 9atom, there's some stuff from Nemo that's derived from nix,
> there's nxm, which is also nix-derived, and there's the pc64 kernel in
> 9front. The nix kernels are divergent from the original kernel. They sit
> in /sys/src/nix instead of /sys/src/9. Pc64 is a straight port of the
> "standard" /sys/src/9/pc kernel.
>
> Pc64 uses 4k pages and exposes the same system calls and interfaces like
> pc. The nix kernels come with 2MB-1GB pages, new system calls and new
> ways of doing things (like the use of segment registers). This port
> targets both pc64 and nix-derived kernels. It doesn't use the crazy
> stuff from the nix kernels.
>
>
> https://codereview.appspot.com/101510049/
>

[aram, 2014-06-30 17:24:46]

On Mon, Jun 30, 2014 at 7:23 PM, Russ Cox <rsc@golang.org> wrote:
> And where is pc64?

https://code.google.com/p/plan9front/source/browse/sys/src/9/pc64

-- 
Aram Hăvărneanu

[aram, 2014-06-30 20:46:37]

https://codereview.appspot.com/101510049/diff/40001/src/pkg/runtime/memclr_pl...
File src/pkg/runtime/memclr_plan9_amd64.s (right):

https://codereview.appspot.com/101510049/diff/40001/src/pkg/runtime/memclr_pl...
src/pkg/runtime/memclr_plan9_amd64.s:13: SHRQ	$3, CX
Ugh, this can't be right, fixing.

[rsc, 2014-06-30 21:29:56]

LGTM

Okay, I'll let this in because there is already plan9/amd64 code in the tree. 

But please do plan for an arm64 builder.

[bradfitz, 2014-06-30 21:31:45]

He's already gotten builder keys from me for both amd64 and arm.
 On Jun 30, 2014 2:29 PM, <rsc@golang.org> wrote:

> LGTM
>
> Okay, I'll let this in because there is already plan9/amd64 code in the
> tree.
>
> But please do plan for an arm64 builder.
>
>
> https://codereview.appspot.com/101510049/
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[dave_cheney.net, 2014-07-01 23:24:47]

Some minor comments.

https://codereview.appspot.com/101510049/diff/40001/src/liblink/obj6.c
File src/liblink/obj6.c (right):

https://codereview.appspot.com/101510049/diff/40001/src/liblink/obj6.c#newcod...
src/liblink/obj6.c:106: //	case Hlinux:
please remove this commented out line

https://codereview.appspot.com/101510049/diff/40001/src/pkg/runtime/arch_amd64.h
File src/pkg/runtime/arch_amd64.h (right):

https://codereview.appspot.com/101510049/diff/40001/src/pkg/runtime/arch_amd6...
src/pkg/runtime/arch_amd64.h:17: #else
gross. Not for this review, but this pattern has reached its limit, is it
possible to improve how this constant is defined ?

https://codereview.appspot.com/101510049/diff/40001/src/pkg/runtime/memclr_pl...
File src/pkg/runtime/memclr_plan9_amd64.s (right):

https://codereview.appspot.com/101510049/diff/40001/src/pkg/runtime/memclr_pl...
src/pkg/runtime/memclr_plan9_amd64.s:13: SHRQ	$3, CX
On 2014/06/30 20:46:37, aram wrote:
> Ugh, this can't be right, fixing.

Do you need to address this before submitting ?

https://codereview.appspot.com/101510049/diff/40001/src/pkg/runtime/os_plan9.c
File src/pkg/runtime/os_plan9.c (right):

https://codereview.appspot.com/101510049/diff/40001/src/pkg/runtime/os_plan9....
src/pkg/runtime/os_plan9.c:160: // be opened twice but that's okay.
this can potentially end up with GOMAXPROCS worth of fd's dedicated to this. I
guess that is ok if you're prepared to live with it.

[aram, 2014-07-01 23:32:21]

https://codereview.appspot.com/101510049/diff/40001/src/liblink/obj6.c
File src/liblink/obj6.c (right):

https://codereview.appspot.com/101510049/diff/40001/src/liblink/obj6.c#newcod...
src/liblink/obj6.c:106: //	case Hlinux:
On 2014/07/01 23:24:47, dfc wrote:
> please remove this commented out line

Done.

https://codereview.appspot.com/101510049/diff/40001/src/pkg/runtime/arch_amd64.h
File src/pkg/runtime/arch_amd64.h (right):

https://codereview.appspot.com/101510049/diff/40001/src/pkg/runtime/arch_amd6...
src/pkg/runtime/arch_amd64.h:17: #else
Done in subsequent CL.

https://codereview.appspot.com/101510049/diff/40001/src/pkg/runtime/memclr_pl...
File src/pkg/runtime/memclr_plan9_amd64.s (right):

https://codereview.appspot.com/101510049/diff/40001/src/pkg/runtime/memclr_pl...
src/pkg/runtime/memclr_plan9_amd64.s:13: SHRQ	$3, CX
It's fine for now.

I'm replacing it with the common version once I change signal handling code.

https://codereview.appspot.com/101510049/diff/40001/src/pkg/runtime/os_plan9.c
File src/pkg/runtime/os_plan9.c (right):

https://codereview.appspot.com/101510049/diff/40001/src/pkg/runtime/os_plan9....
src/pkg/runtime/os_plan9.c:160: // be opened twice but that's okay.
I already changed this code, we use the new nsec system call for both amd64 and
386. Will remove this in a subsequent CL.

[aram, 2014-07-01 23:33:12]

Btw, we have builder now. Also, tests are almost passing:
http://paste.ubuntu.com/7732055/

[dave_cheney.net, 2014-07-02 11:04:29]

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

liblink, runtime: preliminary support for plan9/amd64

A TLS slot is reserved by _rt0_.*_plan9 as an automatic and
its address (which is static on Plan 9) is saved in the
global _privates symbol. The startup linkage now is exactly
like that from Plan 9 libc, and the way we access g is
exactly as if we'd have used privalloc(2).

Aside from making the code more standard, this change
drastically simplifies it, both for 386 and for amd64, and
makes the Plan 9 code in liblink common for both 386 and
amd64.

The amd64 runtime code was cleared of nxm assumptions, and
now runs on the standard Plan 9 kernel.

Note handling fixes will follow in a separate CL.

LGTM=rsc
R=golang-codereviews, rsc, bradfitz, dave
CC=0intro, ality, golang-codereviews, jas, minux.ma, mischief
https://codereview.appspot.com/101510049

Committer: Dave Cheney <dave@cheney.net>

[gobot, 2014-07-02 12:05:50]

This CL appears to have broken the plan9-386-cnielsen builder.
See http://build.golang.org/log/c8b0320fa782202286714778f9b7cbb521fc7fd1

[aram, 2014-07-02 12:25:50]

On Wed, Jul 2, 2014 at 2:05 PM,  <gobot@golang.org> wrote:
> This CL appears to have broken the plan9-386-cnielsen builder.
> See http://build.golang.org/log/c8b0320fa782202286714778f9b7cbb521fc7fd1

No, false positive.

-- 
Aram Hăvărneanu

[ality, 2014-07-07 03:00:21]

LGTM

I know this has already been submitted but I've only
now had time to review it.

Thanks for CC-ing me.

[aram, 2014-07-07 13:11:33]

*** Abandoned ***