code review 6498092: 6l/8l: emit correct opcodes to F(SUB|DIV)R?D.

6l/8l: emit correct opcodes to F(SUB|DIV)R?D.

When the destination was not F0, 6l and 8l swapped FSUBD/FSUBRD and
FDIVD/FDIVRD.

[agl1, 2012-09-05 15:30:37]

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

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

[dave_cheney.net, 2012-09-05 23:35:24]

Hi agl,

Are you able to include a test case with this proposal? 

Cheers

Dave

On 06/09/2012, at 1:30, agl@golang.org wrote:

> Reviewers: golang-dev_googlegroups.com,
> 
> Message:
> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com,
> rsc@golang.org),
> 
> I'd like you to review this change to
> https://code.google.com/p/go/
> 
> 
> Description:
> 6l/8l: emit correct opcodes to F(SUB|DIV)R?D.
> 
> When the destination was not F0, 6l and 8l swapped FSUBD/FSUBRD and
> FDIVD/FDIVRD.
> 
> Please review this at http://codereview.appspot.com/6498092/
> 
> Affected files:
>  M src/cmd/6l/optab.c
>  M src/cmd/8l/optab.c
> 
> 
> Index: src/cmd/6l/optab.c
> ===================================================================
> --- a/src/cmd/6l/optab.c
> +++ b/src/cmd/6l/optab.c
> @@ -1199,25 +1199,25 @@
>    { AFSUBW,    yfmvx,    Px, 0xde,(04) },
>    { AFSUBL,    yfmvx,    Px, 0xda,(04) },
>    { AFSUBF,    yfmvx,    Px, 0xd8,(04) },
> -    { AFSUBD,    yfadd,    Px, 0xdc,(04),0xd8,(04),0xdc,(05) },
> +    { AFSUBD,    yfadd,    Px, 0xdc,(04),0xd8,(04),0xdc,(04) },
> 
>    { AFSUBRDP,    yfaddp,    Px, 0xde,(04) },
>    { AFSUBRW,    yfmvx,    Px, 0xde,(05) },
>    { AFSUBRL,    yfmvx,    Px, 0xda,(05) },
>    { AFSUBRF,    yfmvx,    Px, 0xd8,(05) },
> -    { AFSUBRD,    yfadd,    Px, 0xdc,(05),0xd8,(05),0xdc,(04) },
> +    { AFSUBRD,    yfadd,    Px, 0xdc,(05),0xd8,(05),0xdc,(05) },
> 
>    { AFDIVDP,    yfaddp,    Px, 0xde,(07) },
>    { AFDIVW,    yfmvx,    Px, 0xde,(06) },
>    { AFDIVL,    yfmvx,    Px, 0xda,(06) },
>    { AFDIVF,    yfmvx,    Px, 0xd8,(06) },
> -    { AFDIVD,    yfadd,    Px, 0xdc,(06),0xd8,(06),0xdc,(07) },
> +    { AFDIVD,    yfadd,    Px, 0xdc,(06),0xd8,(06),0xdc,(06) },
> 
>    { AFDIVRDP,    yfaddp,    Px, 0xde,(06) },
>    { AFDIVRW,    yfmvx,    Px, 0xde,(07) },
>    { AFDIVRL,    yfmvx,    Px, 0xda,(07) },
>    { AFDIVRF,    yfmvx,    Px, 0xd8,(07) },
> -    { AFDIVRD,    yfadd,    Px, 0xdc,(07),0xd8,(07),0xdc,(06) },
> +    { AFDIVRD,    yfadd,    Px, 0xdc,(07),0xd8,(07),0xdc,(07) },
> 
>    { AFXCHD,    yfxch,    Px, 0xd9,(01),0xd9,(01) },
>    { AFFREE },
> Index: src/cmd/8l/optab.c
> ===================================================================
> --- a/src/cmd/8l/optab.c
> +++ b/src/cmd/8l/optab.c
> @@ -651,25 +651,25 @@
>    { AFSUBW,    yfmvx,    Px, 0xde,(04) },
>    { AFSUBL,    yfmvx,    Px, 0xda,(04) },
>    { AFSUBF,    yfmvx,    Px, 0xd8,(04) },
> -    { AFSUBD,    yfadd,    Px, 0xdc,(04),0xd8,(04),0xdc,(05) },
> +    { AFSUBD,    yfadd,    Px, 0xdc,(04),0xd8,(04),0xdc,(04) },
> 
>    { AFSUBRDP,    yfaddp,    Px, 0xde,(04) },
>    { AFSUBRW,    yfmvx,    Px, 0xde,(05) },
>    { AFSUBRL,    yfmvx,    Px, 0xda,(05) },
>    { AFSUBRF,    yfmvx,    Px, 0xd8,(05) },
> -    { AFSUBRD,    yfadd,    Px, 0xdc,(05),0xd8,(05),0xdc,(04) },
> +    { AFSUBRD,    yfadd,    Px, 0xdc,(05),0xd8,(05),0xdc,(05) },
> 
>    { AFDIVDP,    yfaddp,    Px, 0xde,(07) },
>    { AFDIVW,    yfmvx,    Px, 0xde,(06) },
>    { AFDIVL,    yfmvx,    Px, 0xda,(06) },
>    { AFDIVF,    yfmvx,    Px, 0xd8,(06) },
> -    { AFDIVD,    yfadd,    Px, 0xdc,(06),0xd8,(06),0xdc,(07) },
> +    { AFDIVD,    yfadd,    Px, 0xdc,(06),0xd8,(06),0xdc,(06) },
> 
>    { AFDIVRDP,    yfaddp,    Px, 0xde,(06) },
>    { AFDIVRW,    yfmvx,    Px, 0xde,(07) },
>    { AFDIVRL,    yfmvx,    Px, 0xda,(07) },
>    { AFDIVRF,    yfmvx,    Px, 0xd8,(07) },
> -    { AFDIVRD,    yfadd,    Px, 0xdc,(07),0xd8,(07),0xdc,(06) },
> +    { AFDIVRD,    yfadd,    Px, 0xdc,(07),0xd8,(07),0xdc,(07) },
> 
>    { AFXCHD,    yfxch,    Px, 0xd9,(01),0xd9,(01) },
>    { AFFREE },
> 
> 

[agl1, 2012-09-08 20:31:04]

On Wed, Sep 5, 2012 at 7:35 PM, Dave Cheney <dave@cheney.net> wrote:
> Are you able to include a test case with this proposal?

I'm sure that rsc will tell me what I've done wrong, but I've added a
test case that fails without this change.


Cheers

AGL

[rsc, 2012-09-10 16:31:12]

LGTM

Good enough. Really it should be rundir but that doesn't exist yet. Will file a
bug to add it.

[agl1, 2012-09-10 19:35:58]

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

6l/8l: emit correct opcodes to F(SUB|DIV)R?D.

When the destination was not F0, 6l and 8l swapped FSUBD/FSUBRD and
FDIVD/FDIVRD.

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