[dev.garbage] code review 173250043: all: merge dev.cc into dev.garbage

all: merge dev.cc into dev.garbage

The garbage collector is now written in Go.
There is plenty to clean up (just like on dev.cc).

all.bash passes on darwin/amd64, darwin/386, linux/amd64, linux/386.

[rsc, 2014-11-15 04:17:36]

Hello austin, rlh (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to the dev.garbage branch of
https://code.google.com/p/go/

[bradfitz, 2014-11-15 04:42:21]

Good ol' chunk mismatch problem.

The one interesting file doesn't show:
https://codereview.appspot.com/173250043/diff/30001/src/runtime/mgc.go

Fortunately our days with hg and rietveld are numbered! Yay! On to new
problems!



On Fri, Nov 14, 2014 at 8:17 PM, <rsc@golang.org> wrote:

> Reviewers: austin, rlh,
>
> Message:
> Hello austin, rlh (cc: golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to the dev.garbage branch of
> https://code.google.com/p/go/
>
>
> Description:
> all: merge dev.cc into dev.garbage
>
> The garbage collector is now written in Go.
> There is plenty to clean up (just like on dev.cc).
>
> all.bash passes on darwin/amd64, darwin/386, linux/amd64, linux/386.
>
> Please review this at https://codereview.appspot.com/173250043/
>
> Affected files (+18691, -79743 lines):
>   M dev.cc
>   M include/link.h
>   R src/cmd/5c/Makefile
>   R src/cmd/5c/cgen.c
>   R src/cmd/5c/doc.go
>   R src/cmd/5c/gc.h
>   R src/cmd/5c/list.c
>   R src/cmd/5c/mul.c
>   R src/cmd/5c/peep.c
>   R src/cmd/5c/reg.c
>   R src/cmd/5c/sgen.c
>   R src/cmd/5c/swt.c
>   R src/cmd/5c/txt.c
>   R src/cmd/6c/Makefile
>   R src/cmd/6c/cgen.c
>   R src/cmd/6c/div.c
>   R src/cmd/6c/doc.go
>   R src/cmd/6c/gc.h
>   R src/cmd/6c/list.c
>   R src/cmd/6c/machcap.c
>   R src/cmd/6c/mul.c
>   R src/cmd/6c/peep.c
>   R src/cmd/6c/reg.c
>   R src/cmd/6c/sgen.c
>   R src/cmd/6c/swt.c
>   R src/cmd/6c/txt.c
>   R src/cmd/8c/Makefile
>   R src/cmd/8c/cgen.c
>   R src/cmd/8c/cgen64.c
>   R src/cmd/8c/div.c
>   R src/cmd/8c/doc.go
>   R src/cmd/8c/gc.h
>   R src/cmd/8c/list.c
>   R src/cmd/8c/machcap.c
>   R src/cmd/8c/mul.c
>   R src/cmd/8c/peep.c
>   R src/cmd/8c/reg.c
>   R src/cmd/8c/sgen.c
>   R src/cmd/8c/swt.c
>   R src/cmd/8c/txt.c
>   R src/cmd/cc/Makefile
>   R src/cmd/cc/acid.c
>   R src/cmd/cc/bits.c
>   R src/cmd/cc/cc.h
>   R src/cmd/cc/cc.y
>   R src/cmd/cc/com.c
>   R src/cmd/cc/com64.c
>   R src/cmd/cc/dcl.c
>   R src/cmd/cc/doc.go
>   R src/cmd/cc/dpchk.c
>   R src/cmd/cc/funct.c
>   R src/cmd/cc/godefs.c
>   R src/cmd/cc/lex.c
>   R src/cmd/cc/mac.c
>   R src/cmd/cc/omachcap.c
>   R src/cmd/cc/pgen.c
>   R src/cmd/cc/pswt.c
>   R src/cmd/cc/scon.c
>   R src/cmd/cc/sub.c
>   R src/cmd/cc/y.tab.h
>   R src/cmd/cc/y.tab.c
>   M src/cmd/cgo/main.go
>   M src/cmd/cgo/out.go
>   M src/cmd/dist/build.c
>   M src/cmd/dist/buildruntime.c
>   M src/cmd/gc/export.c
>   M src/cmd/gc/go.h
>   M src/cmd/gc/lex.c
>   M src/cmd/gc/obj.c
>   M src/cmd/gc/reflect.c
>   M src/cmd/gc/subr.c
>   M src/cmd/go/build.go
>   M src/liblink/asm6.c
>   M src/liblink/objfile.c
>   M src/reflect/type.go
>   M src/run.bash
>   M src/runtime/alg.go
>   M src/runtime/arch1_386.go
>   M src/runtime/arch1_amd64.go
>   M src/runtime/arch1_arm.go
>   R src/runtime/arch_386.h
>   R src/runtime/arch_amd64.h
>   R src/runtime/arch_arm.h
>   M src/runtime/asm.s
>   M src/runtime/asm_386.s
>   M src/runtime/asm_amd64.s
>   M src/runtime/asm_amd64p32.s
>   M src/runtime/asm_arm.s
>   R src/runtime/atomic.go
>   R src/runtime/atomic_386.c
>   M src/runtime/atomic_386.go
>   R src/runtime/atomic_amd64x.c
>   M src/runtime/atomic_amd64x.go
>   M src/runtime/atomic_arm.go
>   M src/runtime/cgo.go
>   R src/runtime/cgo/callbacks.c
>   M src/runtime/cgo/callbacks.go
>   R src/runtime/cgo/dragonfly.c
>   M src/runtime/cgo/dragonfly.go
>   R src/runtime/cgo/freebsd.c
>   M src/runtime/cgo/freebsd.go
>   R src/runtime/cgo/iscgo.c
>   M src/runtime/cgo/iscgo.go
>   R src/runtime/cgo/netbsd.c
>   M src/runtime/cgo/netbsd.go
>   R src/runtime/cgo/openbsd.c
>   M src/runtime/cgo/openbsd.go
>   R src/runtime/cgo/setenv.c
>   M src/runtime/cgo/setenv.go
>   M src/runtime/cgocall.go
>   R src/runtime/chan.h
>   M src/runtime/chan.go
>   M src/runtime/chan1.go
>   M src/runtime/complex.go
>   M src/runtime/cpuprof.go
>   M src/runtime/cputicks.go
>   M src/runtime/debug.go
>   R src/runtime/defs.c
>   M src/runtime/defs1_netbsd_386.go
>   M src/runtime/defs1_netbsd_amd64.go
>   M src/runtime/defs1_netbsd_arm.go
>   M src/runtime/defs1_solaris_amd64.go
>   R src/runtime/defs_android_arm.h
>   R src/runtime/defs_darwin_386.h
>   M src/runtime/defs_darwin_386.go
>   R src/runtime/defs_darwin_amd64.h
>   M src/runtime/defs_darwin_amd64.go
>   R src/runtime/defs_dragonfly_386.h
>   M src/runtime/defs_dragonfly_386.go
>   R src/runtime/defs_dragonfly_amd64.h
>   M src/runtime/defs_dragonfly_amd64.go
>   R src/runtime/defs_freebsd_386.h
>   M src/runtime/defs_freebsd_386.go
>   R src/runtime/defs_freebsd_amd64.h
>   M src/runtime/defs_freebsd_amd64.go
>   R src/runtime/defs_freebsd_arm.h
>   M src/runtime/defs_freebsd_arm.go
>   R src/runtime/defs_linux_386.h
>   M src/runtime/defs_linux_386.go
>   R src/runtime/defs_linux_amd64.h
>   M src/runtime/defs_linux_amd64.go
>   R src/runtime/defs_linux_arm.h
>   M src/runtime/defs_linux_arm.go
>   R src/runtime/defs_nacl_386.h
>   M src/runtime/defs_nacl_386.go
>   R src/runtime/defs_nacl_amd64p32.h
>   M src/runtime/defs_nacl_amd64p32.go
>   R src/runtime/defs_nacl_arm.h
>   M src/runtime/defs_nacl_arm.go
>   R src/runtime/defs_netbsd_386.h
>   R src/runtime/defs_netbsd_amd64.h
>   R src/runtime/defs_netbsd_arm.h
>   R src/runtime/defs_openbsd_386.h
>   M src/runtime/defs_openbsd_386.go
>   R src/runtime/defs_openbsd_amd64.h
>   M src/runtime/defs_openbsd_amd64.go
>   R src/runtime/defs_plan9_386.h
>   M src/runtime/defs_plan9_386.go
>   R src/runtime/defs_plan9_amd64.h
>   M src/runtime/defs_plan9_amd64.go
>   R src/runtime/defs_solaris_amd64.h
>   R src/runtime/defs_windows_386.h
>   M src/runtime/defs_windows_386.go
>   R src/runtime/defs_windows_amd64.h
>   M src/runtime/defs_windows_amd64.go
>   M src/runtime/env_posix.go
>   M src/runtime/export_test.go
>   M src/runtime/extern.go
>   R src/runtime/float.c
>   M src/runtime/funcdata.h
>   M src/runtime/futex_test.go
>   M src/runtime/gcinfo_test.go
>   M src/runtime/go_tls.h
>   src/runtime/heapdump.c
>   M src/runtime/heapdump.go
>   src/runtime/lfstack.c
>   M src/runtime/lfstack.go
>   M src/runtime/lfstack_32bit.go
>   M src/runtime/lfstack_amd64.go
>   M src/runtime/lfstack_linux_power64x.go
>   M src/runtime/lock_futex.go
>   M src/runtime/lock_sema.go
>   src/runtime/malloc.h
>   src/runtime/malloc.c
>   M src/runtime/malloc.go
>   M src/runtime/malloc1.go
>   M src/runtime/malloc2.go
>   src/runtime/mcache.c
>   M src/runtime/mcache.go
>   R src/runtime/mcentral.c
>   M src/runtime/mcentral.go
>   M src/runtime/mem.go
>   M src/runtime/mem_bsd.go
>   R src/runtime/mem_darwin.c
>   M src/runtime/mem_darwin.go
>   R src/runtime/mem_dragonfly.c
>   R src/runtime/mem_freebsd.c
>   src/runtime/mem_linux.c
>   M src/runtime/mem_linux.go
>   R src/runtime/mem_netbsd.c
>   [[ 192 additional files ]]
>
>
> --
> 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.
>

[rsc, 2014-11-15 13:00:48]

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

[dev.garbage] all: merge dev.cc into dev.garbage

The garbage collector is now written in Go.
There is plenty to clean up (just like on dev.cc).

all.bash passes on darwin/amd64, darwin/386, linux/amd64, linux/386.

TBR=rlh
R=austin, rlh, bradfitz
CC=golang-codereviews
https://codereview.appspot.com/173250043

[rsc, 2014-11-15 13:01:10]

I tried uploading again and still got 'bad data' or 'chiunk mismatch', so
this is not reviewable. Will submit it and people can review it locally.

[gobot, 2014-11-15 13:01:38]

This CL appears to have broken the linux-amd64-noopt builder.
See http://build.golang.org/log/33b5160dc878c6817ed6c25f2041712430c522b4

[bradfitz, 2014-11-15 16:40:50]

I liked this part:

func scanstack(gp *g) {
        // TODO(rsc): Due to a precedence error, this was never checked in
the original C version.
        // If you enable the check, the gothrow happens.
/*
                if readgstatus(gp)&_Gscan == 0 {
                        print("runtime: gp=", gp, ", goid=", gp.goid, ",
gp->atomicstatus=", readgstatus(gp), "\n")
                        gothrow("mark - bad status")
                }
        */


On Sat, Nov 15, 2014 at 5:01 AM, Russ Cox <rsc@golang.org> wrote:

> I tried uploading again and still got 'bad data' or 'chiunk mismatch', so
> this is not reviewable. Will submit it and people can review it locally.
>
>

[austin, 2014-11-17 19:36:30]

LGTM

I read mgc0.c -> mgc.go pretty closely.  I only skimmed the rest.

https://codereview.appspot.com/173250043/diff/50001/src/runtime/mgc.go
File src/runtime/mgc.go (left):

https://codereview.appspot.com/173250043/diff/50001/src/runtime/mgc.go#oldcod...
src/runtime/mgc.go:587: 
Lost comment?

https://codereview.appspot.com/173250043/diff/50001/src/runtime/mgc.go#oldcod...
src/runtime/mgc.go:1942: 
Lost comment

https://codereview.appspot.com/173250043/diff/50001/src/runtime/mgc.go
File src/runtime/mgc.go (right):

https://codereview.appspot.com/173250043/diff/50001/src/runtime/mgc.go#newcod...
src/runtime/mgc.go:413: 
Missing printunlock

https://codereview.appspot.com/173250043/diff/50001/src/runtime/mgc.go#newcod...
src/runtime/mgc.go:1475: 
Ewww.

https://codereview.appspot.com/173250043/diff/50001/src/runtime/mgc.go#newcod...
src/runtime/mgc.go:1652: 
In some places this test is still == (or !=) bitBoundary and in others this CL
changed it to == (or !=) 0.  (Either way it's safe because bitBoundary is only a
single bit, but we should be consistent.)

https://codereview.appspot.com/173250043/diff/50001/src/runtime/mgc.go#newcod...
src/runtime/mgc.go:2149: 
Use addb, like above?  Likewise for a few more prog adds below.

https://codereview.appspot.com/173250043/diff/50001/src/runtime/rt0_linux_386.s
File src/runtime/rt0_linux_386.s (left):

https://codereview.appspot.com/173250043/diff/50001/src/runtime/rt0_linux_386...
src/runtime/rt0_linux_386.s:12: CALL	runtime·linux_setup_vdso(SB)
I was surprised to see this in the merge (both parents have this call).  I think
this is fine, because linux_setup_vdso is now called sysargs and rt0_go should
call sysargs (via args) on all platforms, but I'm not positive.