LGTM

http://codereview.chromium.org/115707/diff/18/1006
File test/cctest/test-assembler-x64.cc (right):

http://codereview.chromium.org/115707/diff/18/1006#newcode43
Line 43: // The AMD64 calling convention is used, with the first five
arguments
"first six arguments"

http://codereview.chromium.org/115707/diff/18/1006#newcode45
Line 45: // the XMM registers.  The return value is in RAX.
To be pedantic, if one of the first six arguments is a floating point
value, it's put in the XMM register, and the seventh argument ends up in
R9.
I think it's safer to just say that we don't use floating point
arguments, exclamation point.

http://codereview.chromium.org/115707/diff/18/1006#newcode47
Line 47: // convention is used on 64-bit windows.
The MSVC calling convention uses RCX, RDX, R8 and R9 for arguments -
meaning the test below will not work there.
Add a TODO saying that until we fix on a way to specify parameters.

http://codereview.chromium.org/115707/diff/18/1006#newcode56
Line 56: TEST(AssemblerX640) {
X640 sounds large. Try giving it a more meaningful name than "0".

http://codereview.chromium.org/115707/diff/18/1006#newcode59
Line 59: byte* buffer =
static_cast<byte*>(OS::Allocate(Assembler::kMinimalBufferSize,
Don't use "minimal buffer size" here without asserting that it's large
enough for what you plan to do.
It's better to use some arbitrary, but large enough, size, since the
assembler can't grow it anyway.
And test that the actual size is large enough too.

http://codereview.chromium.org/115707

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to