LGTM.
http://codereview.chromium.org/114085/diff/2005/1008
File src/x64/assembler-x64.cc (right):
http://codereview.chromium.org/114085/diff/2005/1008#newcode902
Line 902: ASSERT(!Heap::InNewSpace(*value));
Is this from the ia32 code? I didn't know this. It makes sense, though
- we can't fix
I've only looked closely at the entry stub. It LGTM.
http://codereview.chromium.org/114085
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Addressed review comments. Please re-review.
http://codereview.chromium.org/114085/diff/1/4
File src/x64/assembler-x64.cc (right):
http://codereview.chromium.org/114085/diff/1/4#newcode76
Line 76:
On 2009/06/08 14:03:46, William Hesse wrote:
> Move the other constructor of Operand to here, from
Small round of comments. Generally: this code needs to be better
commented (I know that IA32 code isn't either, but if you've done the
work to read and understand it, you should preserve it here).
http://codereview.chromium.org/114085/diff/1/6
File src/x64/builtins-x64.cc (right):
http://coder
http://codereview.chromium.org/114085/diff/1/4
File src/x64/assembler-x64.cc (right):
http://codereview.chromium.org/114085/diff/1/4#newcode76
Line 76:
Move the other constructor of Operand to here, from -inl.h
http://codereview.chromium.org/114085/diff/1/4#newcode85
Line 85: set_modrm(0, rsp);
Drive-bys.
http://codereview.chromium.org/114085/diff/1/6
File src/x64/builtins-x64.cc (right):
http://codereview.chromium.org/114085/diff/1/6#newcode79
Line 79: __ movq(rbx, Operand(rbp, 0));
Consider an Operand constructor that takes just the register, no 0.
That (its absence) has been a sour