code review 135070043: runtime: convert runtime1.goc, noasm_arm.goc to Go

runtime: convert runtime1.goc, noasm_arm.goc to Go

[rsc, 2014-08-28 04:13:58]

Hello golang-codereviews@googlegroups.com (cc: iant, khr),

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

[bradfitz, 2014-08-28 05:16:10]

https://codereview.appspot.com/135070043/diff/40001/src/pkg/runtime/extern.go
File src/pkg/runtime/extern.go (right):

https://codereview.appspot.com/135070043/diff/40001/src/pkg/runtime/extern.go...
src/pkg/runtime/extern.go:89: // We assume that all architectures turn faults
and the like
this comment is a little confusing. I think I understand it, but the text on its
own doesn't help much.

https://codereview.appspot.com/135070043/diff/40001/src/pkg/runtime/noasm_arm.go
File src/pkg/runtime/noasm_arm.go (right):

https://codereview.appspot.com/135070043/diff/40001/src/pkg/runtime/noasm_arm...
src/pkg/runtime/noasm_arm.go:6: // but are implemented in Go for arm.
:)

https://codereview.appspot.com/135070043/diff/40001/src/pkg/runtime/noasm_arm...
src/pkg/runtime/noasm_arm.go:21: return +1
the plus sign here and few below seem odd and unnecessary.

if the concern was the look of 1 and l, I'd prefer to see l spelled ln.

[dvyukov, 2014-08-28 07:06:52]

LGTM

[rsc, 2014-08-28 14:46:39]

https://codereview.appspot.com/135070043/diff/40001/src/pkg/runtime/extern.go
File src/pkg/runtime/extern.go (right):

https://codereview.appspot.com/135070043/diff/40001/src/pkg/runtime/extern.go...
src/pkg/runtime/extern.go:89: // We assume that all architectures turn faults
and the like
On 2014/08/28 05:16:10, bradfitz wrote:
> this comment is a little confusing. I think I understand it, but the text on
its
> own doesn't help much.

This is just copied from another file. I'm not changing text in this CL.

https://codereview.appspot.com/135070043/diff/40001/src/pkg/runtime/noasm_arm.go
File src/pkg/runtime/noasm_arm.go (right):

https://codereview.appspot.com/135070043/diff/40001/src/pkg/runtime/noasm_arm...
src/pkg/runtime/noasm_arm.go:21: return +1
On 2014/08/28 05:16:10, bradfitz wrote:
> the plus sign here and few below seem odd and unnecessary.
> 
> if the concern was the look of 1 and l, I'd prefer to see l spelled ln.

This is a standard thing for comparison functions: they return -1, 0, or +1.

[rsc, 2014-08-28 14:47:05]

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

runtime: convert runtime1.goc, noasm_arm.goc to Go

LGTM=dvyukov
R=golang-codereviews, bradfitz, dvyukov
CC=golang-codereviews, iant, khr
https://codereview.appspot.com/135070043

[gobot, 2014-08-28 14:48:05]

This CL appears to have broken the android-arm-crawshaw builder.
See http://build.golang.org/log/aaef5b3dfab8813824fe29489da6e39d4f73f854

[dave_cheney.net, 2014-08-28 14:49:34]

yup, this is real


On Fri, Aug 29, 2014 at 12:48 AM,  <gobot@golang.org> wrote:
> This CL appears to have broken the android-arm-crawshaw builder.
> See http://build.golang.org/log/aaef5b3dfab8813824fe29489da6e39d4f73f854
>
>
> https://codereview.appspot.com/135070043/
>
> --
> 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-08-28 15:44:43]

took me a while but it's fixed.

[0intro, 2014-08-29 18:22:47]

On Plan 9, we get:

fatal error: getenv before env init

goroutine 1 [running, locked to thread]:
runtime.gothrow(0x178b48, 0x16)
	/usr/go/src/pkg/runtime/panic.c:527 +0x7c fp=0x103c1f00 sp=0x103c1ef0
runtime.gogetenv(0x1604a8, 0x6, 0x0, 0x0)
	/usr/go/src/pkg/runtime/extern.go:157 +0x67 fp=0x103c1f4c sp=0x103c1f00
runtime.GOROOT(0x0, 0x0)
	/usr/go/src/pkg/runtime/extern.go:171 +0x42 fp=0x103c1f60 sp=0x103c1f4c
cmd/gofmt.init()
	/usr/go/src/cmd/gofmt/long_test.go:27 +0x3b4 fp=0x103c1f94 sp=0x103c1f60
main.init()
	cmd/gofmt/_test/_testmain.go:52 +0x41 fp=0x103c1f98 sp=0x103c1f94
runtime.main()
	/usr/go/src/pkg/runtime/proc.c:251 +0xb3 fp=0x103c1fd0 sp=0x103c1f98
runtime.goexit()
	/usr/go/src/pkg/runtime/proc.c:1625 fp=0x103c1fd4 sp=0x103c1fd0

goroutine 2 [runnable]:
runtime.func·001()
	/usr/go/src/pkg/runtime/proc.go:34 fp=0x1037bfd0 sp=0x1037bfcc
runtime.goexit()
	/usr/go/src/pkg/runtime/proc.c:1625 fp=0x1037bfd4 sp=0x1037bfd0
created by runtime.init·2
	/usr/go/src/pkg/runtime/proc.go:50 +0x27

goroutine 3 [runnable]:
bgsweep()
	/usr/go/src/pkg/runtime/mgc0.c:1055 fp=0x10377fd0 sp=0x10377fcc
runtime.goexit()
	/usr/go/src/pkg/runtime/proc.c:1625 fp=0x10377fd4 sp=0x10377fd0
created by gc
	/usr/go/src/pkg/runtime/mgc0.c:1305

goroutine 4 [runnable]:
runtime.runfinq()
	/usr/go/src/pkg/runtime/malloc.go:692 fp=0x10361fd0 sp=0x10361fcc
runtime.goexit()
	/usr/go/src/pkg/runtime/proc.c:1625 fp=0x10361fd4 sp=0x10361fd0
created by runtime.createfing
	/usr/go/src/pkg/runtime/malloc.go:687 +0x5b
FAIL	cmd/gofmt	0.041s

[0intro, 2014-08-29 19:14:15]

It happens because runtime/os_plan9.c:/^runtime·goenvs isn't implemented.