X64: Implementation of a bunch of stubs, and some new opcodes.

X64: Implementation of a bunch of stubs, and some new opcodes.

[Lasse Reichstein, 2009-06-16 13:11:25]

Largish review.

[William Hesse, 2009-06-17 08:27:48]

LGTM.

The emit(0x40 ...) is the same as emit_rex_32(..), so use emit_rex_32( .. )

The other one might be an emit_modrm().

http://codereview.chromium.org/125185/diff/1/4
File src/x64/assembler-x64.cc (right):

http://codereview.chromium.org/125185/diff/1/4#newcode1232
Line 1232: emit(0x40 | (dst.code() & 0x8) >> 1| (src.code() & 0x8) >> 3);
Is this really right?  Not a REX prefix?

http://codereview.chromium.org/125185/diff/1/5
File src/x64/assembler-x64.h (right):

http://codereview.chromium.org/125185/diff/1/5#newcode478
Line 478: void movzxq_b(Register dst, const Operand& src);
I thought the suffixes were going to be in src, dst order, so that move from
byte to quadword would be movzxbq.  Is this notation from an Intel or gcc
standard?  If, we will adopt it, but comment up where the b,w,l,q suffixes are
explained.

http://codereview.chromium.org/125185/diff/1/5#newcode814
Line 814: 
What is our strategy for floating point?  Does MMX give us transcendentals, and
SSE2 give us basic arithmetic?

http://codereview.chromium.org/125185/diff/1/6
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/125185/diff/1/6#newcode347
Line 347: 
I've been sticking with single spacing between these, until they become real
implementations.  Then I double space.  I suppose it doesn't matter, but it
should be uniform.

http://codereview.chromium.org/125185/diff/1/6#newcode453
Line 453: // TODO(x64): Don't use fp stack, use MMX registers?
OK, so we port existing code, and make the shift to SSE2 a future TODO - that
sounds good.

http://codereview.chromium.org/125185/diff/1/6#newcode554
Line 554: __ addq(rax, rbx);  // add optimistically
I think we want addl here.  If our smis are zero-extended 32-bit smis, then we
want the overflow flag to indicate an overflow into bit 33.  I think we can make
this code work for 32-bit smis, on x64.

http://codereview.chromium.org/125185/diff/1/6#newcode559
Line 559: __ subq(rax, rbx);  // subtract optimistically
subl

http://codereview.chromium.org/125185/diff/1/6#newcode670
Line 670: __ cmpq(rax, Immediate(0xc0000000));
cmpl, and many previous instructions should be changed.

http://codereview.chromium.org/125185/diff/1/6#newcode858
Line 858: __ ret(0);
Ret() is a MacroAssembler shortcut for ret(0).

[Lasse Reichstein, 2009-06-17 09:37:11]

http://codereview.chromium.org/125185/diff/1/4
File src/x64/assembler-x64.cc (right):

http://codereview.chromium.org/125185/diff/1/4#newcode1232
Line 1232: emit(0x40 | (dst.code() & 0x8) >> 1| (src.code() & 0x8) >> 3);
It is a REX prefix, but the current methods expect Register, not MMXRegister, so
they didn't apply.
If we become serious about adding MMX instructions, we'll need emit-rex's for
MMX registers too. For now, I have removed it all.

http://codereview.chromium.org/125185/diff/1/5
File src/x64/assembler-x64.h (right):

http://codereview.chromium.org/125185/diff/1/5#newcode478
Line 478: void movzxq_b(Register dst, const Operand& src);
This was just a bad adoptation of the ia32 notation.
I'll change it to movzxbq (readability is ... not so impressive, though :)
Eventually, I guess we'll have an mxyzptlk instruction for something.

http://codereview.chromium.org/125185/diff/1/5#newcode814
Line 814: 
For now we just use plain old f-operations. I'll remove these again, since I
didn't use them anyway.

http://codereview.chromium.org/125185/diff/1/6
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/125185/diff/1/6#newcode554
Line 554: __ addq(rax, rbx);  // add optimistically
On 2009/06/17 08:27:48, William Hesse wrote:
> I think we want addl here.  If our smis are zero-extended 32-bit smis, then we
> want the overflow flag to indicate an overflow into bit 33.  I think we can
make
> this code work for 32-bit smis, on x64.

Smis are sign-extended 32-bit numbers.

http://codereview.chromium.org/125185/diff/1/6#newcode670
Line 670: __ cmpq(rax, Immediate(0xc0000000));
On 2009/06/17 08:27:48, William Hesse wrote:
> cmpl, and many previous instructions should be changed.

Should be cmpq, to get sign extension on the immediate.

http://codereview.chromium.org/125185/diff/1/6#newcode858
Line 858: __ ret(0);
Changed.
Although, somehow, I find ret(0) more readable than Ret(). Guess the capital
letter stands out.

[William Hesse, 2009-06-17 11:31:37]

http://codereview.chromium.org/125185/diff/1/6
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/125185/diff/1/6#newcode670
Line 670: __ cmpq(rax, Immediate(0xc0000000));
On 2009/06/17 09:37:11, Lasse Reichstein wrote:
> On 2009/06/17 08:27:48, William Hesse wrote:
> > cmpl, and many previous instructions should be changed.
> 
> Should be cmpq, to get sign extension on the immediate.

No, we need both negative and positive numbers to become negative by subtracting
this negative number.  Only a 32-bit addition of this 32-bit number will work,
that sets the sign flag based on bit 31 of the result.  cmpl is the correct
instruction, that takes 32-bit operands, does the 32-bit math, and sets the bits
based on the 32-bit result, then stores the zero-extended result in the register
(but cmp does not store its result).  For addl and subl, where you want the
result to be sign-extended, do a real sign-extension operation after the tests. 
It may be that zero-extended smis and sign-extended smis can be mixed together,
if we only every look at their low 32 bits.

[Lasse Reichstein, 2009-06-17 11:37:10]

New version uploaded, please recheck.

http://codereview.chromium.org/125185/diff/1/6
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/125185/diff/1/6#newcode670
Line 670: __ cmpq(rax, Immediate(0xc0000000));
Ah, yes, we use the overflow flag. That won't work unless we do 32-bit
operations.
I'll upload a new version.

[William Hesse, 2009-06-17 11:40:34]

LGTM.