Add a stub for allocating parameter maps.

Add a stub for allocating parameter maps.

TODO:
- Two remaining test case failures that Kevin will look into:
  * One due to an UNIMPLEMENTED.
  * One assertion failure in debug/serdes_ExternalWrap__nocrankshaft that
    I don't quite understand.
- x64 and ARM backends.
Committed: http://code.google.com/p/v8/source/detail?r=7960

[fschneider, 2011-04-27 08:38:02]

redirecting to Kevin.

[Kevin Millikin (Chromium), 2011-04-27 11:49:28]

LGTM.  Feel free to treat the comments below as TODO's (i.e., commit this and
you or I can clean up the huge chunk of assembly code when we get a chance).

http://codereview.chromium.org/6824081/diff/11001/src/code-stubs.h
File src/code-stubs.h (right):

http://codereview.chromium.org/6824081/diff/11001/src/code-stubs.h#newcode690
src/code-stubs.h:690: int GetArgumentsBoilerplateIndex() const {
I think this function and the next (GetArgumentsObjectSize) are no longer
needed.

http://codereview.chromium.org/6824081/diff/11001/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):

http://codereview.chromium.org/6824081/diff/11001/src/ia32/code-stubs-ia32.cc...
src/ia32/code-stubs-ia32.cc:3671: __ mov(edx, Operand(ebp,
StandardFrameConstants::kCallerFPOffset));
I think some bit of this code is shared by all three GenerateNew... functions. 
We should probably put it in a separate helper.

http://codereview.chromium.org/6824081/diff/11001/src/ia32/code-stubs-ia32.cc...
src/ia32/code-stubs-ia32.cc:3698: static const int kDisplacement = 2 *
kPointerSize;
It strikes me that kDisplacement is really just
StandardFrameConstants::kCallerSPOffset (though that's a longer name it might be
clearer to use it here).

Also, this whole code is actually off by one by pointing to the receiver, which
we don't need.  So we could adjust by using
StandardFrameConstants::kCallerSPOffset.  That might also be clearer.

http://codereview.chromium.org/6824081/diff/11001/src/ia32/code-stubs-ia32.cc...
src/ia32/code-stubs-ia32.cc:3701: __ mov(ebx, Operand(esp, 1 * kPointerSize));
We have just pushed this before the call.  We could consider passing it in a
register instead (we would possibly have to spill it eventually).

http://codereview.chromium.org/6824081/diff/11001/src/ia32/code-stubs-ia32.cc...
src/ia32/code-stubs-ia32.cc:3717: __ mov(ecx, Operand(edx,
ArgumentsAdaptorFrameConstants::kLengthOffset));
Also, instead of computing the non-adaptor receiver address in the caller, and
patching it for the adaptor case in the callee, we could compute both cases in
the callee---it might be clearer.

http://codereview.chromium.org/6824081/diff/11001/src/ia32/code-stubs-ia32.cc...
src/ia32/code-stubs-ia32.cc:3749: __ add(Operand(ebx),
Immediate(GetArgumentsObjectSize()));
I think the size is a constant here.

http://codereview.chromium.org/6824081/diff/11001/src/ia32/code-stubs-ia32.cc...
src/ia32/code-stubs-ia32.cc:3839: __ push(ecx);
It's unfortunate that the parameters are in the reverse order, that's due to
trying to avoid unneeded context slots for ones that are shadowed by other
parameters.

There may be a way to avoid this push (by using some trick that allows us to use
the index to compute addresses in the parameter map and backing store from some
'phantom' base address), but I doubt the performance is worth the extra
complication.

[rossberg, 2011-04-27 23:18:00]

PTAL. I did some of the easier modifications you proposed.

http://codereview.chromium.org/6824081/diff/11001/src/code-stubs.h
File src/code-stubs.h (right):

http://codereview.chromium.org/6824081/diff/11001/src/code-stubs.h#newcode690
src/code-stubs.h:690: int GetArgumentsBoilerplateIndex() const {
On 2011/04/27 11:49:28, Kevin Millikin wrote:
> I think this function and the next (GetArgumentsObjectSize) are no longer
> needed.

Done.

http://codereview.chromium.org/6824081/diff/11001/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):

http://codereview.chromium.org/6824081/diff/11001/src/ia32/code-stubs-ia32.cc...
src/ia32/code-stubs-ia32.cc:3671: __ mov(edx, Operand(ebp,
StandardFrameConstants::kCallerFPOffset));
On 2011/04/27 11:49:28, Kevin Millikin wrote:
> I think some bit of this code is shared by all three GenerateNew... functions.

> We should probably put it in a separate helper.

Put in a TODO.

http://codereview.chromium.org/6824081/diff/11001/src/ia32/code-stubs-ia32.cc...
src/ia32/code-stubs-ia32.cc:3698: static const int kDisplacement = 2 *
kPointerSize;
On 2011/04/27 11:49:28, Kevin Millikin wrote:
> It strikes me that kDisplacement is really just
> StandardFrameConstants::kCallerSPOffset (though that's a longer name it might
be
> clearer to use it here).

Done.

> Also, this whole code is actually off by one by pointing to the receiver,
which
> we don't need.  So we could adjust by using
> StandardFrameConstants::kCallerSPOffset.  That might also be clearer.

I'm not entirely sure what you are referring to here.

http://codereview.chromium.org/6824081/diff/11001/src/ia32/code-stubs-ia32.cc...
src/ia32/code-stubs-ia32.cc:3701: __ mov(ebx, Operand(esp, 1 * kPointerSize));
On 2011/04/27 11:49:28, Kevin Millikin wrote:
> We have just pushed this before the call.  We could consider passing it in a
> register instead (we would possibly have to spill it eventually).

Does it matter much?

http://codereview.chromium.org/6824081/diff/11001/src/ia32/code-stubs-ia32.cc...
src/ia32/code-stubs-ia32.cc:3717: __ mov(ecx, Operand(edx,
ArgumentsAdaptorFrameConstants::kLengthOffset));
On 2011/04/27 11:49:28, Kevin Millikin wrote:
> Also, instead of computing the non-adaptor receiver address in the caller, and
> patching it for the adaptor case in the callee, we could compute both cases in
> the callee---it might be clearer.

True. I leave that for later.

http://codereview.chromium.org/6824081/diff/11001/src/ia32/code-stubs-ia32.cc...
src/ia32/code-stubs-ia32.cc:3749: __ add(Operand(ebx),
Immediate(GetArgumentsObjectSize()));
On 2011/04/27 11:49:28, Kevin Millikin wrote:
> I think the size is a constant here.

Done (here and elsewhere).

http://codereview.chromium.org/6824081/diff/11001/src/ia32/code-stubs-ia32.cc...
src/ia32/code-stubs-ia32.cc:3839: __ push(ecx);
On 2011/04/27 11:49:28, Kevin Millikin wrote:
> It's unfortunate that the parameters are in the reverse order, that's due to
> trying to avoid unneeded context slots for ones that are shadowed by other
> parameters.
> 
> There may be a way to avoid this push (by using some trick that allows us to
use
> the index to compute addresses in the parameter map and backing store from
some
> 'phantom' base address), but I doubt the performance is worth the extra
> complication.

Yes, I felt that clarity was more important here.

[Kevin Millikin (Chromium), 2011-05-19 08:18:58]

LGTM, go ahead and land it.

[Kevin Millikin (Chromium), 2011-05-19 12:04:18]

+karlklose