LGTM
http://codereview.chromium.org/147205/diff/1/2 File src/x64/builtins-x64.cc (right): http://codereview.chromium.org/147205/diff/1/2#newcode185 Line 185: // +1 ~ return address. Not understood. The +1 is added because of the return address? Make a constant and refer to the stack structure. http://codereview.chromium.org/147205/diff/1/2#newcode316 Line 316: __ subq(rcx, Operand(kScratchRegister, 0)); // We need this number below. Make a comment that the value in "rcx" is reused later. http://codereview.chromium.org/147205/diff/1/2#newcode316 Line 316: __ subq(rcx, Operand(kScratchRegister, 0)); // We need this number below. Reword comment. Not clear what "number" refers to, or what "need" means. After the subq, rcx contains the location of rsp relative to the stack guard limit. If the limit's above the stack pointer, we are have hit the limit and must take action. http://codereview.chromium.org/147205/diff/1/2#newcode320 Line 320: // Because builtins always remove the receiver from the stack, we It's a runtime function, not a builtin (well, not the way V8 usually uses "builtin") http://codereview.chromium.org/147205/diff/1/2#newcode356 Line 356: __ movq(rdi, Operand(rbp, 4 * kPointerSize)); Could you add a telling name for rbp + 4 * kPointerSize, if one does not exist already. http://codereview.chromium.org/147205/diff/1/2#newcode361 Line 361: __ movq(rbx, Operand(rbp, 3 * kPointerSize)); And for rbp + 3 * kPointerSize http://codereview.chromium.org/147205 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
