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), On 2010/10/21 11:52:38, antonm wrote:
as you introduce kFirstApiParameter constant anyway, maybe add MoveApiParemeter(int index, Operand) macro instead?
Added Operand ApiParameterOperand(int index) 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; On 2010/10/21 11:52:38, antonm wrote:
could we move labels closer to the place they are used?
Done. 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)); On 2010/10/21 11:52:38, antonm wrote:
I don't think you need to explicitly give reloc info here
Done. 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)); On 2010/10/21 11:52:38, antonm wrote:
Please, add a comment that you load a pointer to output handle here
Done. 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. On 2010/10/21 11:52:38, antonm wrote:
couldn't you use mov(Operand(eax, 0), Immediate(0)); instead?
Sure we could. It would save one byte of code but wouldn't it introduce data dependence? 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 On 2010/10/21 11:52:38, antonm wrote:
overthere -> over there, or better there or just omit it: Prepares
stack for
arguments.
Done. 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). On 2010/10/21 11:52:38, antonm wrote:
needs -> needed
Done. 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 On 2010/10/21 11:52:38, antonm wrote:
calle<e>
Done. 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 On 2010/10/21 11:52:38, antonm wrote:
What I miss: I see no tail call to API function?
It actually does call + ret that logical equivalent to jump. http://codereview.chromium.org/3792003/diff/123001/80018#newcode497 src/ia32/macro-assembler-ia32.h:497: // returned value from handle and propogates exceptions. On 2010/10/21 11:52:38, antonm wrote:
prop<a>gates
Done. 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. On 2010/10/21 11:52:38, antonm wrote:
it seems to be callee save on both Win64 and Linux, no?
Done. 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). On 2010/10/21 11:52:38, antonm wrote:
Same as for ia32
Done. http://codereview.chromium.org/3792003/diff/123001/80025#newcode824 src/x64/macro-assembler-x64.h:824: // stored in esp[0], esp[4] etc. On 2010/10/21 11:52:38, antonm wrote:
this appears to be an incorrect copy paste from former ia32 doc
Done. http://codereview.chromium.org/3792003/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
