Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(110)

Issue 2273041: code review 2273041: Initial Plan9 runtime support for 386. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 1 month ago by paulzhol
Modified:
7 years ago
Reviewers:
CC:
rsc, golang-dev
Visibility:
Public.

Description

Initial Plan9 runtime support for 386. No multiple processes/locks, managed to compile and run a hello.go (with print not fmt). Also test/sieve.go seems to run until 439 and stops with a 'throw: all goroutines are asleep - deadlock!' - just like runtime/tiny. based on Russes suggestions at: http://groups.google.com/group/comp.os.plan9/browse_thread/thread/cfda8b82535d2d68/243777a597ec1612 (I'm not sure I can use the 0xdffffefc address like that but it seems to run...) Build instructions: cd src/pkg/runtime make clean && GOOS=plan9 make install this will build and install the runtime. When linking with 8l, you should pass -s to suppress symbol generation in the a.out, otherwise the generated executable will not run. This is runtime only, the porting of the toolchain has already been done: http://code.google.com/p/go-plan9/source/browse in the plan9-quanstro branch.

Patch Set 1 : code review 2273041: Initial Plan9 runtime support for 386. #

Total comments: 12

Patch Set 2 : code review 2273041: Initial Plan9 runtime support for 386. #

Patch Set 3 : code review 2273041: Initial Plan9 runtime support for 386. #

Patch Set 4 : code review 2273041: Initial Plan9 runtime support for 386 [update to release.2010-09-29]. #

Patch Set 5 : code review 2273041: Initial Plan9 runtime support for 386. #

Patch Set 6 : code review 2273041: Initial Plan9 runtime support for 386. #

Total comments: 11

Patch Set 7 : code review 2273041: Initial Plan9 runtime support for 386. #

Total comments: 2

Patch Set 8 : code review 2273041: Initial Plan9 runtime support for 386. #

Total comments: 6

Patch Set 9 : code review 2273041: Initial Plan9 runtime support for 386. #

Patch Set 10 : code review 2273041: Initial Plan9 runtime support for 386. #

Patch Set 11 : code review 2273041: Initial Plan9 runtime support for 386. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+370 lines, -7 lines) Patch
M src/Make.inc View 3 4 5 6 7 8 9 1 chunk +2 lines, -1 line 0 comments Download
M src/cmd/8l/asm.c View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
M src/cmd/8l/obj.c View 1 2 3 4 5 6 7 8 9 10 1 chunk +3 lines, -0 lines 0 comments Download
M src/cmd/8l/pass.c View 1 2 3 4 5 6 7 8 9 10 2 chunks +16 lines, -1 line 0 comments Download
M src/pkg/runtime/386/asm.s View 3 4 5 6 7 8 2 chunks +3 lines, -1 line 0 comments Download
M src/pkg/runtime/mkasmh.sh View 1 2 3 1 chunk +5 lines, -0 lines 0 comments Download
A src/pkg/runtime/plan9/386/defs.h View 1 1 chunk +1 line, -0 lines 0 comments Download
A src/pkg/runtime/plan9/386/rt0.s View 1 2 3 4 5 6 7 8 9 1 chunk +32 lines, -0 lines 0 comments Download
A src/pkg/runtime/plan9/386/signal.c View 1 chunk +10 lines, -0 lines 0 comments Download
A src/pkg/runtime/plan9/386/sys.s View 1 2 3 4 5 6 7 8 9 1 chunk +76 lines, -0 lines 0 comments Download
A src/pkg/runtime/plan9/mem.c View 1 2 3 4 5 6 7 8 9 1 chunk +49 lines, -0 lines 0 comments Download
A src/pkg/runtime/plan9/os.h View 1 2 3 4 5 6 7 8 9 1 chunk +27 lines, -0 lines 0 comments Download
A src/pkg/runtime/plan9/signals.h View 1 chunk +1 line, -0 lines 0 comments Download
A src/pkg/runtime/plan9/thread.c View 1 2 3 4 5 6 7 8 9 1 chunk +135 lines, -0 lines 0 comments Download
M src/pkg/runtime/runtime.h View 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/runtime/runtime.c View 3 4 5 6 7 8 9 1 chunk +8 lines, -3 lines 0 comments Download

Messages

Total messages: 23
paulzhol
Hello golang-dev@googlegroups.com, I'd like you to review this change.
7 years, 1 month ago (2010-09-23 17:54:11 UTC) #1
paulzhol
Hi, I've tried to get a basic google go runtime for plan9 the (b) part ...
7 years, 1 month ago (2010-09-23 23:11:08 UTC) #2
rsc1
Very cool! You changed the TLS story while I was halfway through the review. I ...
7 years, 1 month ago (2010-09-24 22:58:47 UTC) #3
paulzhol
Hi! I saw some Plan 9 src/9/prot/sysproc.c code for the exec? syscall, and a struct ...
7 years, 1 month ago (2010-09-25 22:31:16 UTC) #4
rsc
> My hello go works but the sieve is giving me: > throw: mark - ...
7 years ago (2010-09-28 16:08:29 UTC) #5
paulzhol
> That suggests there are references to invalid > goroutine states. If you find that ...
7 years ago (2010-09-29 22:21:18 UTC) #6
rsc
> term% sieve > newproc1 0x5eb4(<-mainstart) 0xdfffef20 narg=0 nret=0 > goid=1 > gp: 0x3f000 gp->status:2(<-Grunning) ...
7 years ago (2010-09-30 01:55:47 UTC) #7
paulzhol
> > The global variable g should never have g->status == Gidle. > I suspect ...
7 years ago (2010-09-30 15:11:44 UTC) #8
paulzhol
Update: Got newosproc working with rfork, implemented locking with plan9 semaphores (Sape Mullender's paper), also ...
7 years ago (2010-10-06 08:12:42 UTC) #9
paulzhol
> I did notice a something strange: without the Paranoia checking in > src/pkg/runtime/plan9/386/sys.s (Patchset ...
7 years ago (2010-10-07 06:59:14 UTC) #10
rsc
On Thu, Oct 7, 2010 at 02:59, <paulzhol@gmail.com> wrote: >> I did notice a something ...
7 years ago (2010-10-07 07:06:26 UTC) #11
rsc1
Looks pretty good. http://codereview.appspot.com/2273041/diff/93001/src/pkg/runtime/386/asm.s File src/pkg/runtime/386/asm.s (right): http://codereview.appspot.com/2273041/diff/93001/src/pkg/runtime/386/asm.s#newcode408 src/pkg/runtime/386/asm.s:408: TEXT tlscheck(SB), 7, $0 GLOBL isplan9(SB), ...
7 years ago (2010-10-07 08:12:41 UTC) #12
paulzhol
http://codereview.appspot.com/2273041/diff/2001/src/cmd/8l/pass.c File src/cmd/8l/pass.c (right): http://codereview.appspot.com/2273041/diff/2001/src/cmd/8l/pass.c#newcode687 src/cmd/8l/pass.c:687: case 2: // Plan9 On 2010/09/24 22:58:47, rsc1 wrote: ...
7 years ago (2010-10-07 22:52:23 UTC) #13
paulzhol
Hi Russ, Thank you for the CR, I have updated my patches. Pavel
7 years ago (2010-10-07 22:54:43 UTC) #14
rsc
On Thu, Oct 7, 2010 at 18:52, <paulzhol@gmail.com> wrote: > > http://codereview.appspot.com/2273041/diff/2001/src/cmd/8l/pass.c > File src/cmd/8l/pass.c ...
7 years ago (2010-10-11 01:11:49 UTC) #15
paulzhol
Hi Russ, I've changed rt0.s a bit to make it more readable, I'm not sure ...
7 years ago (2010-10-11 07:48:10 UTC) #16
rsc1
LGTM Can take or leave the rt0.s comment but please fix the jump in asm.s. ...
7 years ago (2010-10-12 02:50:08 UTC) #17
paulzhol
ok, updated. http://codereview.appspot.com/2273041/diff/102001/src/pkg/runtime/386/asm.s File src/pkg/runtime/386/asm.s (right): http://codereview.appspot.com/2273041/diff/102001/src/pkg/runtime/386/asm.s#newcode30 src/pkg/runtime/386/asm.s:30: JZ ok On 2010/10/12 02:50:08, rsc1 wrote: ...
7 years ago (2010-10-12 08:06:59 UTC) #18
rsc
Sorry for dragging this out, but could you please hg sync hg upload 2273041 ? ...
7 years ago (2010-10-12 13:40:01 UTC) #19
rsc
Also it appears you haven't completed the CLA as described at http://golang.org/doc/contribute.html#copyright . Please do. ...
7 years ago (2010-10-12 13:41:30 UTC) #20
paulzhol
I've updated the patches against tip and signed the individual CLA. Thanks for your patience, ...
7 years ago (2010-10-12 22:15:09 UTC) #21
paulzhol
synced against 6537:31e328e0c972.
7 years ago (2010-10-16 08:17:47 UTC) #22
rsc
7 years ago (2010-10-18 16:32:59 UTC) #23
*** Submitted as 8d16ce8ee935 ***

8l, runtime: initial support for Plan 9

No multiple processes/locks, managed to compile
and run a hello.go (with print not fmt).  Also test/sieve.go
seems to run until 439 and stops with a
'throw: all goroutines are asleep - deadlock!'
- just like runtime/tiny.

based on Russ's suggestions at:
http://groups.google.com/group/comp.os.plan9/browse_thread/thread/cfda8b82535...

Build instructions:
cd src/pkg/runtime
make clean && GOOS=plan9 make install
this will build and install the runtime.

When linking with 8l, you should pass -s to suppress symbol
generation in the a.out, otherwise the generated executable will not run.

This is runtime only, the porting of the toolchain has already
been done: http://code.google.com/p/go-plan9/source/browse
in the plan9-quanstro branch.

R=rsc
CC=golang-dev
http://codereview.appspot.com/2273041

Committer: Russ Cox <rsc@golang.org>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 80a51fa-tainted