X64: Call runtime function from JS stack frame.

X64: Call runtime function from JS stack frame.

[Lasse Reichstein, 2009-06-12 12:27:18]

Medium size review.

[William Hesse, 2009-06-12 12:45:04]

LGTM.

http://codereview.chromium.org/125049/diff/1/2
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/125049/diff/1/2#newcode432
Line 432: // rax: result parameter for PerformGC, if any.
What does "result parameter" mean?  This is really just the first (only)
parameter that PerformGC takes?  But why is it on the stack?  Or is it the
return address that a GC call should return to?

http://codereview.chromium.org/125049/diff/1/2#newcode436
Line 436: // rdi: number of arguments including receiver.
rdi is also callee-saved.  Maybe specify the Unix/Windows X64 ABI in a comment
above // rax.

http://codereview.chromium.org/125049/diff/1/2#newcode450
Line 450: __ movq(kScratchRegister, scope_depth);
I have also used this code sequence for Statistics counters.  Should we have a
MacroAssembler::Incl(ExternalReference) and
Decl(ExternalReference)?

http://codereview.chromium.org/125049/diff/1/2#newcode462
Line 462: // First arguments is already in rdi.
argument

http://codereview.chromium.org/125049/diff/1/2#newcode596
Line 596: // rdi: number of arguments including receiver.
Why isn't this marked as C callee-saved?  Or if it isn't anymore (because it is
now an argument register), then shouldn't it be put in a callee-saved one?

[Lasse Reichstein, 2009-06-12 13:09:13]

http://codereview.chromium.org/125049/diff/1/2
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/125049/diff/1/2#newcode432
Line 432: // rax: result parameter for PerformGC, if any.
After the code of GenerateCore, the following code is run only if a failure
happened. In that case, rax holds the resulting failure value. If GenerateCore
is given a true value as do_gc argument, it is used as the parameter to
Runtime::PerformGC.
It is unset until after the first GenerateCore (unless GLAG_gc_greedy is set).
So it's both a result of the C-call and parameter to PerformGC.

http://codereview.chromium.org/125049/diff/1/2#newcode436
Line 436: // rdi: number of arguments including receiver.
The rdi register is only callee-save in the Windows X64 ABI, not the Unix X64
ABI (it's a parameter register). We should treat it as volatile except in
MSVC-specific code.
Added comment to that point.

http://codereview.chromium.org/125049/diff/1/2#newcode450
Line 450: __ movq(kScratchRegister, scope_depth);
If that's the only two places, I'm not too keen on doing extra functions for it.
But it is an idea.

http://codereview.chromium.org/125049/diff/1/2#newcode462
Line 462: // First arguments is already in rdi.
fixed.

http://codereview.chromium.org/125049/diff/1/2#newcode596
Line 596: // rdi: number of arguments including receiver.
It's not callee-saved in the Unix 64-bit ABI, but the values doesn't need to be
saved across the C call anyway, so I kept the value in the parameter register
and removed the "callee-saved" comment.

[William Hesse, 2009-06-12 14:56:50]

http://codereview.chromium.org/125049/diff/1/2
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/125049/diff/1/2#newcode432
Line 432: // rax: result parameter for PerformGC, if any.
On 2009/06/12 13:09:13, Lasse Reichstein wrote:
> After the code of GenerateCore, the following code is run only if a failure
> happened. In that case, rax holds the resulting failure value. If GenerateCore
> is given a true value as do_gc argument, it is used as the parameter to
> Runtime::PerformGC.
> It is unset until after the first GenerateCore (unless GLAG_gc_greedy is set).
> So it's both a result of the C-call and parameter to PerformGC.

I would comment it as:
// rax: result of the previous call to the function.  Pass it to PerformGC if a
GC is needed.

http://codereview.chromium.org/125049/diff/1/2#newcode450
Line 450: __ movq(kScratchRegister, scope_depth);
On 2009/06/12 13:09:13, Lasse Reichstein wrote:
> If that's the only two places, I'm not too keen on doing extra functions for
it.
> But it is an idea.

Well, they are also the only places incl and decl are used.