code review 5989057: runtime: cgo support for Linux/ARM

runtime: cgo support for Linux/ARM
        Part 2 of CL 5601044 (cgo: Linux/ARM support)

[minux1, 2012-04-07 19:28:01]

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/

[dave_cheney.net, 2012-04-09 08:09:55]

LGTM - tested on Pandaboard ARMv7

[dave_cheney.net, 2012-04-09 10:31:45]

On 2012/04/09 08:09:55, dfc wrote:
> LGTM - tested on Pandaboard ARMv7

Looks good on ARMv5 as well

[rsc, 2012-04-10 20:25:06]

I found the TLS stuff I guess.  That's too much assembly for today; will try to
read it again tomorrow.

I'm very glad you're doing this and not me.  :-)
Thank you.

[rsc, 2012-04-25 02:25:19]

Pretty good, just some confusion on my part.

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/asm_arm.s
File src/pkg/runtime/asm_arm.s (right):

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/asm_arm.s#new...
src/pkg/runtime/asm_arm.s:280: BIC	$0x7, R13	// alignment for gcc ABI
It hardly matters but you might want to look at the disassembly in gdb or 5l -a.
 I think AND $~0x7 assembles cheaper than BIC $0x7 due to the choices for
constants on ARM, but I am not 100% sure.

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/asm_arm.s#new...
src/pkg/runtime/asm_arm.s:301: SUB 	$4, R13 // XXX: how to use "str r4, [sp,
-4]!"?
I believe that is MOVW.P R4, -4(R13)

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/cgo/gcc_arm.S
File src/pkg/runtime/cgo/gcc_arm.S (right):

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/cgo/gcc_arm.S...
src/pkg/runtime/cgo/gcc_arm.S:28: mov lr, pc
What does this do?  My guess would be that it jumps to lr, but I don't know what
lr might have in it right now, and I especially don't know how we get to the
next instruction.

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/cgo/gcc_arm.S...
src/pkg/runtime/cgo/gcc_arm.S:43: push {r0, r1, r2, r4, r5, r6, r7, r8, r9, r10,
r11, ip, lr}
Why are more registers saved here than above? And what happened to r3?

[minux1, 2012-04-25 07:44:29]

Hello dave@cheney.net, rsc@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[minux1, 2012-04-25 07:45:05]

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/asm_arm.s
File src/pkg/runtime/asm_arm.s (right):

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/asm_arm.s#new...
src/pkg/runtime/asm_arm.s:280: BIC	$0x7, R13	// alignment for gcc ABI
On 2012/04/25 02:25:20, rsc wrote:
> It hardly matters but you might want to look at the disassembly in gdb or 5l
-a.
>  I think AND $~0x7 assembles cheaper than BIC $0x7 due to the choices for
> constants on ARM, but I am not 100% sure.
I've checked the output of objdump, and "BIC $0x7, R13" is encoded 0xe3cdd007.

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/asm_arm.s#new...
src/pkg/runtime/asm_arm.s:301: SUB 	$4, R13 // XXX: how to use "str r4, [sp,
-4]!"?
On 2012/04/25 02:25:20, rsc wrote:
> I believe that is MOVW.P R4, -4(R13)
that will be "str     r4, [sp], #-4" as confirmed by objdump.
After some experiments, I finally found out that "str r4, [sp, #-4]!"
should be "MOVW.W R4, -4(SP)".

For the record, these suffixes are defined in ARM instruction
encoding.

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/cgo/gcc_arm.S
File src/pkg/runtime/cgo/gcc_arm.S (right):

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/cgo/gcc_arm.S...
src/pkg/runtime/cgo/gcc_arm.S:28: mov lr, pc
On 2012/04/25 02:25:20, rsc wrote:
> What does this do?  My guess would be that it jumps to lr, but I don't know
what
> lr might have in it right now, and I especially don't know how we get to the
> next instruction.
GNU as(1) is somewhat confusing that its operand order is reversed for ARM.

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/cgo/gcc_arm.S...
src/pkg/runtime/cgo/gcc_arm.S:43: push {r0, r1, r2, r4, r5, r6, r7, r8, r9, r10,
r11, ip, lr}
On 2012/04/25 02:25:20, rsc wrote:
> Why are more registers saved here than above? And what happened to r3?
we still have to save callee save registers (r4-r10, r11,  ip, lr)
and we need to pass two parameter r1, r2 to the procedure,
also at 5a procedure entry, the first argument is at 4(R13) not 0(R13),
so we push r0 for that slot.
Another reason for pushing r0 is that cgo_tls_get_gm will clobber
r0, so we must save r0 somewhere.

Anyway, I've changed the comment to present all the reasoning.

[minux1, 2012-05-03 08:34:13]

ping?

[rsc, 2012-05-03 21:56:19]

LGTM

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/asm_arm.s
File src/pkg/runtime/asm_arm.s (right):

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/asm_arm.s#new...
src/pkg/runtime/asm_arm.s:301: SUB 	$4, R13 // XXX: how to use "str r4, [sp,
-4]!"?
On 2012/04/25 07:45:05, minux wrote:
> On 2012/04/25 02:25:20, rsc wrote:
> > I believe that is MOVW.P R4, -4(R13)
> that will be "str     r4, [sp], #-4" as confirmed by objdump.
> After some experiments, I finally found out that "str r4, [sp, #-4]!"
> should be "MOVW.W R4, -4(SP)".
> 
> For the record, these suffixes are defined in ARM instruction
> encoding.

Did you want to use MOVW.W R4, -4(SP) here instead of the MOVW+SUB?

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/cgo/gcc_arm.S
File src/pkg/runtime/cgo/gcc_arm.S (right):

http://codereview.appspot.com/5989057/diff/4001/src/pkg/runtime/cgo/gcc_arm.S...
src/pkg/runtime/cgo/gcc_arm.S:28: mov lr, pc
On 2012/04/25 07:45:05, minux wrote:
> On 2012/04/25 02:25:20, rsc wrote:
> > What does this do?  My guess would be that it jumps to lr, but I don't know
> what
> > lr might have in it right now, and I especially don't know how we get to the
> > next instruction.
> GNU as(1) is somewhat confusing that its operand order is reversed for ARM.

Aha!  Thanks.  That makes a lot more sense now.

[minux1, 2012-05-04 10:08:41]

On 2012/05/03 21:56:19, rsc wrote:
> Did you want to use MOVW.W R4, -4(SP) here instead of the MOVW+SUB?
Yes, I forgot to update the code here.

[minux1, 2012-05-04 10:20:45]

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

runtime: cgo support for Linux/ARM
        Part 2 of CL 5601044 (cgo: Linux/ARM support)

R=dave, rsc
CC=golang-dev
http://codereview.appspot.com/5989057