code review 118480046: liblink: encode MOVBQZX as MOVZBL instead of MOVZBQ

liblink: encode MOVBQZX as MOVZBL instead of MOVZBQ

[ruiu, 2014-08-04 03:24:43]

Hello golang-codereviews@googlegroups.com,

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

[gobot, 2014-08-05 15:40:11]

R=rsc@golang.org (assigned by r@golang.org)

[rsc, 2014-08-06 03:57:27]

not lgtm

The MOVBQ change is okay, but the others are not safe. They change the
meaning of the instruction. I admit it is in a way that most code will not
care about, but you never know with hand-written assembly, and I'd rather
not surprise people. There are enough surprises here.

A better place for this kind of optimization would be in 6g/peep.c near the
'MOVSD removal'.

Russ

[ruiu, 2014-08-06 04:26:59]

Thank you for reviewing. I agree -- this CL changes the meaning fo *SX
instructions as it wouldn't sign-extend the upper 32 bit.

I updated this CL so that it now alters the encoding of MOVBQZX. PTAL.

On Tue, Aug 5, 2014 at 8:57 PM, Russ Cox <rsc@golang.org> wrote:

> not lgtm
>
> The MOVBQ change is okay, but the others are not safe. They change the
> meaning of the instruction. I admit it is in a way that most code will not
> care about, but you never know with hand-written assembly, and I'd rather
> not surprise people. There are enough surprises here.
>
> A better place for this kind of optimization would be in 6g/peep.c near
> the 'MOVSD removal'.
>
> Russ
>

[rsc, 2014-08-06 04:31:15]

LGTM

[ruiu, 2014-08-06 04:38:43]

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

liblink: encode MOVBQZX as MOVZBL instead of MOVZBQ

LGTM=rsc
R=golang-codereviews, gobot, rsc
CC=golang-codereviews
https://codereview.appspot.com/118480046