code review 204067: 8a/8l: Added CMOVcc instructions

8a/8l: Added CMOVcc instructions

[eds, 2010-02-07 19:20:29]

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

I'd like you to review this change.

[ken3, 2010-02-07 22:36:34]

i have 2 comments about this change.

1. the opcodes are not just enums, but are the
actual opcodes used in the .6 files between 8c/8g and 8l.
thus this change will invalidate ALL .6 files. everything
must be recompiled. normally, updates can be added
to the end after AEND and before ALAST. in this case,
it is not so bad since go is in such flux and most people
recompile everything after every update. i would still
move the new opcodes to the end.

2. if you are going to do this, i would do the same
to 6a/6l.

On Sun, Feb 7, 2010 at 11:20 AM,  <chickencha@gmail.com> wrote:
> Reviewers: rsc,
>
> Message:
> Hello rsc (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change.
>
>
> Description:
> 8a/8l: Added CMOVcc instructions
>
> Note that I made the operand order different from the corresponding
> instructions in 6a/6l because the Plan 9 assembler manual says the order
> should always be source then destination.
>
> Please review this at http://codereview.appspot.com/204067/show
>
> Affected files:
>  M src/cmd/8a/lex.c
>  M src/cmd/8l/8.out.h
>  M src/cmd/8l/optab.c
>
>
>

[eds, 2010-02-08 04:02:55]

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

Please take another look.

[eds, 2010-02-08 04:05:57]

1. Done

2. I planned on doing this in a separate CL, assuming that this is accepted.
Is that okay?

- Evan

On Sun, Feb 7, 2010 at 4:36 PM, Ken Thompson <ken@google.com> wrote:

> i have 2 comments about this change.
>
> 1. the opcodes are not just enums, but are the
> actual opcodes used in the .6 files between 8c/8g and 8l.
> thus this change will invalidate ALL .6 files. everything
> must be recompiled. normally, updates can be added
> to the end after AEND and before ALAST. in this case,
> it is not so bad since go is in such flux and most people
> recompile everything after every update. i would still
> move the new opcodes to the end.
>
> 2. if you are going to do this, i would do the same
> to 6a/6l.
>
> On Sun, Feb 7, 2010 at 11:20 AM,  <chickencha@gmail.com> wrote:
> > Reviewers: rsc,
> >
> > Message:
> > Hello rsc (cc: golang-dev@googlegroups.com),
> >
> > I'd like you to review this change.
> >
> >
> > Description:
> > 8a/8l: Added CMOVcc instructions
> >
> > Note that I made the operand order different from the corresponding
> > instructions in 6a/6l because the Plan 9 assembler manual says the order
> > should always be source then destination.
> >
> > Please review this at http://codereview.appspot.com/204067/show
> >
> > Affected files:
> >  M src/cmd/8a/lex.c
> >  M src/cmd/8l/8.out.h
> >  M src/cmd/8l/optab.c
> >
> >
> >
>

[rsc, 2010-02-08 05:05:41]

I also don't understand your most recent email:
it looks like the instructions are already in 6l.

http://codereview.appspot.com/204067/diff/8/1003
File src/cmd/8l/optab.c (right):

http://codereview.appspot.com/204067/diff/8/1003#newcode583
src/cmd/8l/optab.c:583: { ACMOVLCC,	yml_rl,	Pm, 0x43 },
Your comment says you made the operand order different
than in 6l but I can't see what you mean.

Could you please elaborate?

[eds, 2010-02-08 17:56:53]

The instructions are already in 6l, but the current usage for 6a is:

CMOVLCC destination,source

According to the Plan 9 assembler manual, this operand order seems wrong. I
intended to make the usage in 8a:

CMOVLCC source,destination

But it looks like I messed it up. (I swear it worked before I submitted! ;))
If this is okay, I will revise this CL to use the correct operand order and
submit a separate CL to change the order in 6a/6l. Sorry for the confusion.

- Evan

On Sun, Feb 7, 2010 at 11:05 PM, <rsc@golang.org> wrote:

> I also don't understand your most recent email:
> it looks like the instructions are already in 6l.
>
>
>
> http://codereview.appspot.com/204067/diff/8/1003
> File src/cmd/8l/optab.c (right):
>
> http://codereview.appspot.com/204067/diff/8/1003#newcode583
> src/cmd/8l/optab.c:583: { ACMOVLCC,     yml_rl, Pm, 0x43 },
> Your comment says you made the operand order different
> than in 6l but I can't see what you mean.
>
> Could you please elaborate?
>
>
> http://codereview.appspot.com/204067/show
>

[eds, 2010-02-09 03:09:31]

After looking at this a bit more, I think I'm wrong. The 6a/6l syntax is
correct as it is. I was referencing documents that used Intel syntax, but
was disassembling with an AT&T syntax disassembler, so I got all mixed up.
I've changed the commit comment and this CL can be reviewed as is.

Once again, sorry for all the confusion. I think I've set myself straight
now.

- Evan

On Sun, Feb 7, 2010 at 11:05 PM, <rsc@golang.org> wrote:

> I also don't understand your most recent email:
> it looks like the instructions are already in 6l.
>
>
>
> http://codereview.appspot.com/204067/diff/8/1003
> File src/cmd/8l/optab.c (right):
>
> http://codereview.appspot.com/204067/diff/8/1003#newcode583
> src/cmd/8l/optab.c:583: { ACMOVLCC,     yml_rl, Pm, 0x43 },
> Your comment says you made the operand order different
> than in 6l but I can't see what you mean.
>
> Could you please elaborate?
>
>
> http://codereview.appspot.com/204067/show
>

[rsc, 2010-02-09 21:21:16]

LGTM

[rsc, 2010-02-09 21:33:09]

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

8a/8l: Added CMOVcc instructions

R=rsc
CC=golang-dev
http://codereview.appspot.com/204067

Committer: Russ Cox <rsc@golang.org>