code review 7226066: cmd/8l: fix misassembling of MOVB involving (AX)(BX*1)

cmd/8l: fix misassembling of MOVB involving (AX)(BX*1)

The linker accepts MOVB involving non-byte-addressable
registers, by generating XCHG instructions to AX or BX.
It does not handle the case where nor AX nor BX are available.

See also revision 1470920a2804.

Assembling
    TEXT ·Truc(SB),7,$0
    MOVB BP, (BX)(AX*1)
    RET

gives before:
   08048c60 <main.Truc>:
    8048c60:       87 dd         xchg   %ebx,%ebp
    8048c62:       88 1c 03      mov    %bl,(%ebx,%eax,1)
    8048c65:       87 dd         xchg   %ebx,%ebp
    8048c67:       c3            ret

and after:
   08048c60 <main.Truc>:
    8048c60:       87 cd         xchg   %ecx,%ebp
    8048c62:       88 0c 03      mov    %cl,(%ebx,%eax,1)
    8048c65:       87 cd         xchg   %ecx,%ebp
    8048c67:       c3            ret

[remyoudompheng, 2013-01-30 22:18:41]

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

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc, 2013-01-31 01:33:34]

LGTM

https://codereview.appspot.com/7226066/diff/5001/src/cmd/8l/span.c
File src/cmd/8l/span.c (right):

https://codereview.appspot.com/7226066/diff/5001/src/cmd/8l/span.c#newcode857
src/cmd/8l/span.c:857: diag("impossible");
impossible byte register
(something a little more distinctive)

https://codereview.appspot.com/7226066/diff/5001/src/cmd/8l/span.c#newcode1325
src/cmd/8l/span.c:1325: // We certainly don't want to exchange
How about:

if((breg = byteswapreg(&p->to)) != D_AX) {

and make byteswapreg (formerly isax) handle the p->to.type == D_NONE case.
On the other hand maybe that doesn't work. I don't know why we're skipping AX on
the to.type==D_NONE case anyway.

[rsc, 2013-01-31 01:33:52]

I can only imagine the debugging session that led you to discover this. My
condolences.

[remyoudompheng, 2013-01-31 07:53:11]

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

cmd/8l: fix misassembling of MOVB involving (AX)(BX*1)

The linker accepts MOVB involving non-byte-addressable
registers, by generating XCHG instructions to AX or BX.
It does not handle the case where nor AX nor BX are available.

See also revision 1470920a2804.

Assembling
    TEXT ·Truc(SB),7,$0
    MOVB BP, (BX)(AX*1)
    RET

gives before:
   08048c60 <main.Truc>:
    8048c60:       87 dd         xchg   %ebx,%ebp
    8048c62:       88 1c 03      mov    %bl,(%ebx,%eax,1)
    8048c65:       87 dd         xchg   %ebx,%ebp
    8048c67:       c3            ret

and after:
   08048c60 <main.Truc>:
    8048c60:       87 cd         xchg   %ecx,%ebp
    8048c62:       88 0c 03      mov    %cl,(%ebx,%eax,1)
    8048c65:       87 cd         xchg   %ecx,%ebp
    8048c67:       c3            ret

R=golang-dev, rsc
CC=golang-dev
https://codereview.appspot.com/7226066

https://codereview.appspot.com/7226066/diff/5001/src/cmd/8l/span.c
File src/cmd/8l/span.c (right):

https://codereview.appspot.com/7226066/diff/5001/src/cmd/8l/span.c#newcode857
src/cmd/8l/span.c:857: diag("impossible");
On 2013/01/31 01:33:34, rsc wrote:
> impossible byte register
> (something a little more distinctive)

Done.

https://codereview.appspot.com/7226066/diff/5001/src/cmd/8l/span.c#newcode1325
src/cmd/8l/span.c:1325: // We certainly don't want to exchange
On 2013/01/31 01:33:34, rsc wrote:
> How about:
> 
> if((breg = byteswapreg(&p->to)) != D_AX) {
> 
> and make byteswapreg (formerly isax) handle the p->to.type == D_NONE case.
> On the other hand maybe that doesn't work. I don't know why we're skipping AX
on
> the to.type==D_NONE case anyway.
> 

Done.

(the D_NONE case was for unary MULB/DIVB)