code review 106720044: armasm: add plan 9 output

armasm: add plan 9 output

[rsc, 2014-05-31 14:02:22]

Hello minux,

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

[rsc, 2014-05-31 14:02:27]

*** Submitted as
https://code.google.com/p/rsc/source/detail?r=08f5d599cb70&repo=arm ***

armasm: add plan 9 output

TBR=minux
https://codereview.appspot.com/106720044

[minux, 2014-06-01 08:15:48]

i think we need more special cases. for example,
this plan 9 assembly:
MOVM.IA.W [R0-R7], R8
is disassembled as:
STM [R0-R7], R8!

of course, this depends on how close to the plan 9 syntax
you want to make cmd/objdump.

other issues that I found while playing with cmd/objdump:
1. translate MVN $x, Ra to MOVW $~x, Ra; similar for CMN.
2. translate PUSH and POP to appropriate MOVMs.
3. (probably a problem of the objdump, not the arm disassembler)
all private symbols are not decoded (e.g. udiv<>(SB) in
src/pkg/runtime/vlop_arm.s)

I understand that it's pretty close to 1.3 release and you're
busy fixing the other more important bugs, so please feel
free to defer any of the above issues and comment marked
non-essential below, but please do address the Mem issue.

https://codereview.appspot.com/106720044/diff/1/armasm/plan9x.go
File armasm/plan9x.go (right):

https://codereview.appspot.com/106720044/diff/1/armasm/plan9x.go#newcode44
armasm/plan9x.go:44: if inst.Op&^15 == LDR_EQ && reg == R15 && mem.Base == SP &&
mem.Sign == 0 && mem.Mode == AddrPostIndex {
also need to check for POP [R15] (not essential).

https://codereview.appspot.com/106720044/diff/1/armasm/plan9x.go#newcode124
armasm/plan9x.go:124: op = "MOVB" + op[4:] + suffix
LDRB means load byte with zero extend in ARM docs., whereas MOVB is
load byte with sign extension.
LDRB should translate to MOVBU, and LDRSB should translate to MOVB.

similar for LDR{,S}H.

https://codereview.appspot.com/106720044/diff/1/armasm/plan9x.go#newcode157
armasm/plan9x.go:157: return fmt.Sprintf("$%d", int(a))
why use decimal instead of hexadecimal here?

https://codereview.appspot.com/106720044/diff/1/armasm/plan9x.go#newcode159
armasm/plan9x.go:159: case Mem:
perhaps I'm missing something, but i didn't find where
you handled proper plan 9 translation of Mem operands
for LDRD, SWP, LDM/STM, LDREX/STREX instructions.

why not move the special handling code here?

[minux, 2014-06-01 19:50:24]

LGTM.

[rsc, 2014-06-01 22:51:46]

Thank you for the comments. They somehow didn't arrive in my email. I will
address them after the 1.3 release.