[v8-dev] Re: X64: Implement CEntryStub and JSEntryTrampoline.

2009-06-10 Thread whesse
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

[v8-dev] Re: X64: Implement CEntryStub and JSEntryTrampoline.

2009-06-09 Thread kmillikin
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 -~--~~~~--~~--~--~---

[v8-dev] Re: X64: Implement CEntryStub and JSEntryTrampoline.

2009-06-09 Thread lrn
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

[v8-dev] Re: X64: Implement CEntryStub and JSEntryTrampoline.

2009-06-09 Thread kmillikin
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

[v8-dev] Re: X64: Implement CEntryStub and JSEntryTrampoline.

2009-06-08 Thread whesse
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);

[v8-dev] Re: X64: Implement CEntryStub and JSEntryTrampoline.

2009-06-08 Thread kmillikin
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