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). http://codereview.chromium.org/125185 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
