code review 6034048: sync/atomic: fix 64-bit atomic cas for Linux/ARM

sync/atomic: fix 64-bit atomic cas for Linux/ARM
    This is a follow-up to CL 5978051.
    Use kernel cas64 helper if we can, fallback to LDREXD/STREXD if
    we are on ARMv6 or higher, and to lock-emulated cas64 if on ARMv5.
    A future CL will fix {Add,Load,Store}{Int,Uint}64 and issue 3331.

[minux1, 2012-04-15 20:08:20]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

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

[minux1, 2012-04-19 19:22:43]

any thoughts on this CL and CL 5978051?

another way to get architecture information is by parsing /proc/cpuinfo,
or SIGILL on unsupported instructions, I think neither of them is acceptable.

[bradfitz, 2012-04-19 19:33:53]

What's the concern about parsing /proc/cpuinfo?  Complexity or stability?
 It's Linux-specific, sure, but it's part of the stable interface provided
to userspace.  I wouldn't worry about it changing, for instance.

On Thu, Apr 19, 2012 at 12:22 PM, <minux.ma@gmail.com> wrote:

> any thoughts on this CL and CL 5978051?
>
> another way to get architecture information is by parsing /proc/cpuinfo,
> or SIGILL on unsupported instructions, I think neither of them is
> acceptable.
>
>
http://codereview.appspot.com/**6034048/<http://codereview.appspot.com/6034048/>
>

[minux1, 2012-04-19 19:42:08]

On Fri, Apr 20, 2012 at 3:33 AM, Brad Fitzpatrick <bradfitz@golang.org>wrote:

> What's the concern about parsing /proc/cpuinfo?  Complexity or stability?
>  It's Linux-specific, sure, but it's part of the stable interface provided
> to userspace.  I wouldn't worry about it changing, for instance.

The same info is already provided by AUXV in machine readable form.
AT_PLATFORM and AT_HWCAP,
(as shown by: 'LD_SHOW_AUXV=1 /bin/true')
so I don't think we need to read a file and parse that ourselves.

Also note that the AUXV is stable interface too.

[rsc, 2012-04-25 03:14:10]

http://codereview.appspot.com/6034048/diff/12001/src/pkg/sync/atomic/init_arm.go
File src/pkg/sync/atomic/init_arm.go (right):

http://codereview.appspot.com/6034048/diff/12001/src/pkg/sync/atomic/init_arm...
src/pkg/sync/atomic/init_arm.go:9: func init() {
sync/atomic is imported by just about everything, transitively.
Having a func init here means that every package ends up with a func init (to
call this one), and I'd really like to keep them more lightweight than that.  

The 64-bit detection code in asm_arm.s checks whether it can use the
64-bit operations and caches the result, so that the fastCheck64 function
only executes 3 instructions on the common path.  Can we do something
like that here instead of a func init phase check?

[minux1, 2012-04-25 08:25:55]

Hello bradfitz@golang.org, rsc@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2012-05-03 21:47:43]

http://codereview.appspot.com/6034048/diff/21003/src/pkg/sync/atomic/asm_linu...
File src/pkg/sync/atomic/asm_linux_arm.s (right):

http://codereview.appspot.com/6034048/diff/21003/src/pkg/sync/atomic/asm_linu...
src/pkg/sync/atomic/asm_linux_arm.s:113: MOVB	runtime·armArch(SB), R0
Where is this symbol defined?

[minux1, 2012-05-04 15:19:39]

On 2012/05/03 21:47:43, rsc wrote:
http://codereview.appspot.com/6034048/diff/21003/src/pkg/sync/atomic/asm_linu...
> src/pkg/sync/atomic/asm_linux_arm.s:113: MOVB	runtime·armArch(SB), R0
> Where is this symbol defined?
In CL 5978051, signal_linux_arm.c

[rsc, 2012-05-04 15:20:28]

LGTM

[minux1, 2012-05-04 18:02:53]

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

sync/atomic: fix 64-bit atomic cas for Linux/ARM
    This is a follow-up to CL 5978051.
    Use kernel cas64 helper if we can, fallback to LDREXD/STREXD if
    we are on ARMv6 or higher, and to lock-emulated cas64 if on ARMv5.
    A future CL will fix {Add,Load,Store}{Int,Uint}64 and issue 3331.

R=bradfitz, rsc
CC=golang-dev
http://codereview.appspot.com/6034048