Thanks a lot for refactoring API function invocation.
http://codereview.chromium.org/3792003/diff/123001/80016 File src/ia32/code-stubs-ia32.cc (right): http://codereview.chromium.org/3792003/diff/123001/80016#newcode3064 src/ia32/code-stubs-ia32.cc:3064: __ mov(Operand(esp, (MacroAssembler::kFistApiParameter + 0) * kPointerSize), as you introduce kFirstApiParameter constant anyway, maybe add MoveApiParemeter(int index, Operand) macro instead? http://codereview.chromium.org/3792003/diff/123001/80017 File src/ia32/macro-assembler-ia32.cc (right): http://codereview.chromium.org/3792003/diff/123001/80017#newcode1101 src/ia32/macro-assembler-ia32.cc:1101: Label empty_handle; could we move labels closer to the place they are used? http://codereview.chromium.org/3792003/diff/123001/80017#newcode1119 src/ia32/macro-assembler-ia32.cc:1119: lea(eax, Operand(esp, (argc + 1) * kPointerSize, RelocInfo::NONE)); I don't think you need to explicitly give reloc info here http://codereview.chromium.org/3792003/diff/123001/80017#newcode1119 src/ia32/macro-assembler-ia32.cc:1119: lea(eax, Operand(esp, (argc + 1) * kPointerSize, RelocInfo::NONE)); Please, add a comment that you load a pointer to output handle here http://codereview.chromium.org/3792003/diff/123001/80017#newcode1121 src/ia32/macro-assembler-ia32.cc:1121: mov(Operand(esp, (argc + 1) * kPointerSize), Immediate(0)); // out cell. couldn't you use mov(Operand(eax, 0), Immediate(0)); instead? http://codereview.chromium.org/3792003/diff/123001/80018 File src/ia32/macro-assembler-ia32.h (right): http://codereview.chromium.org/3792003/diff/123001/80018#newcode489 src/ia32/macro-assembler-ia32.h:489: // Prepares stack to put arguments overthere (aligns and so on). Reserves overthere -> over there, or better there or just omit it: Prepares stack for arguments. http://codereview.chromium.org/3792003/diff/123001/80018#newcode490 src/ia32/macro-assembler-ia32.h:490: // space for return value if needs (assumes the return value is a handle). needs -> needed http://codereview.chromium.org/3792003/diff/123001/80018#newcode491 src/ia32/macro-assembler-ia32.h:491: // Uses calle-saved esi to restore stack state after call. Arguments must be calle<e> http://codereview.chromium.org/3792003/diff/123001/80018#newcode496 src/ia32/macro-assembler-ia32.h:496: // Tail call an API function (jump). Allocates HandleScope, extracts What I miss: I see no tail call to API function? http://codereview.chromium.org/3792003/diff/123001/80018#newcode497 src/ia32/macro-assembler-ia32.h:497: // returned value from handle and propogates exceptions. prop<a>gates http://codereview.chromium.org/3792003/diff/123001/80024 File src/x64/macro-assembler-x64.cc (right): http://codereview.chromium.org/3792003/diff/123001/80024#newcode500 src/x64/macro-assembler-x64.cc:500: Register base_reg = kSmiConstantRegister; // callee-save on Win64. it seems to be callee save on both Win64 and Linux, no? http://codereview.chromium.org/3792003/diff/123001/80025 File src/x64/macro-assembler-x64.h (right): http://codereview.chromium.org/3792003/diff/123001/80025#newcode822 src/x64/macro-assembler-x64.h:822: // Prepares stack to put arguments overthere (aligns and so on). Same as for ia32 http://codereview.chromium.org/3792003/diff/123001/80025#newcode824 src/x64/macro-assembler-x64.h:824: // stored in esp[0], esp[4] etc. this appears to be an incorrect copy paste from former ia32 doc http://codereview.chromium.org/3792003/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
