Strict mode function entry (Function.prototype.call/apply)

Strict mode function entry.
Function.call/Function.apply skip transformation of the thisArg
when calling strict mode function.


BUG=
TEST=es5conform, test/mjsunit/strict-mode.js

[Martin Maly, 2011-02-15 05:14:00]

In strict mode, "this" is passed unchanged through calls to
Function.prototype.call and Function.prototype.apply.

There will be further change, to pass this unchanged through regular function
call, outside of Function.prototype.call and Function.prototype.apply. Hopefully
coming soon.

Thank you!
Martin

http://codereview.chromium.org/6524006/diff/1/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):

http://codereview.chromium.org/6524006/diff/1/src/x64/builtins-x64.cc#newcode651
src/x64/builtins-x64.cc:651: // Compute the receiver in non-strict mode.
Tricky ... on 64 bit the fields in SharedFunctionInfo are packed and every other
is not smi. CompilerHints is one of them, so the mask is not shifted by
(kStrictModeFunction + kSmiTagSize) as on arm and ia32.

[Mads Ager (chromium), 2011-02-15 08:09:31]

LGTM

http://codereview.chromium.org/6524006/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/6524006/diff/1/src/objects.h#newcode4390
src/objects.h:4390: public:
I would rather make Builtins a friend or make them all public.

[Lasse Reichstein, 2011-02-15 08:46:55]

LGTM.
This will also allow us to close issue
http://code.google.com/p/v8/issues/detail?id=981

http://codereview.chromium.org/6524006/diff/1/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):

http://codereview.chromium.org/6524006/diff/1/src/ia32/builtins-ia32.cc#newco...
src/ia32/builtins-ia32.cc:752: kSmiTagSize)));
You can test directly against memory, without loading into ecx first, i.e.,
 test(FieldOperand(ecx, SharedFunctionInfo::kCompilerHintsOffset),
Immediate(...));

You might also want to use a byte-test
 ASSERT_EQ(8, SharedFunctionInfo::kStrictModeFunction);
 test_b(FieldOperand(ecx, ... + 1), Immediate((1 << (...)) >> 8);
It would allow an 8-bit immediate instead of a 32-bit one.
(Ditto for X64, with the appropriate differences for encoding).

[Mads Ager (chromium), 2011-02-15 08:50:12]

http://codereview.chromium.org/6524006/diff/1/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):

http://codereview.chromium.org/6524006/diff/1/src/ia32/builtins-ia32.cc#newco...
src/ia32/builtins-ia32.cc:752: kSmiTagSize)));
On 2011/02/15 08:46:55, Lasse Reichstein wrote:
> You can test directly against memory, without loading into ecx first, i.e.,
>  test(FieldOperand(ecx, SharedFunctionInfo::kCompilerHintsOffset),
> Immediate(...));
> 
> You might also want to use a byte-test
>  ASSERT_EQ(8, SharedFunctionInfo::kStrictModeFunction);
>  test_b(FieldOperand(ecx, ... + 1), Immediate((1 << (...)) >> 8);
> It would allow an 8-bit immediate instead of a 32-bit one.
> (Ditto for X64, with the appropriate differences for encoding).

If you do use the byte version, please introduce some named constants to use in
the immediate to make it readable. :-)

[Martin Maly, 2011-02-15 19:18:43]

Made the changes. I like the byte operations so I went with that, the math to
calculate which byte and which bit within it is in object.h, close to where the
smi packing for 64 bit is explained (it also accounts for smi tag). The codegen
looks much cleaner.

Sorry about the presubmit.

Martin

http://codereview.chromium.org/6524006/diff/1/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):

http://codereview.chromium.org/6524006/diff/1/src/ia32/builtins-ia32.cc#newco...
src/ia32/builtins-ia32.cc:752: kSmiTagSize)));
On 2011/02/15 08:50:12, Mads Ager wrote:
> On 2011/02/15 08:46:55, Lasse Reichstein wrote:
> > You can test directly against memory, without loading into ecx first, i.e.,
> >  test(FieldOperand(ecx, SharedFunctionInfo::kCompilerHintsOffset),
> > Immediate(...));
> > 
> > You might also want to use a byte-test
> >  ASSERT_EQ(8, SharedFunctionInfo::kStrictModeFunction);
> >  test_b(FieldOperand(ecx, ... + 1), Immediate((1 << (...)) >> 8);
> > It would allow an 8-bit immediate instead of a 32-bit one.
> > (Ditto for X64, with the appropriate differences for encoding).
> 
> If you do use the byte version, please introduce some named constants to use
in
> the immediate to make it readable. :-)

Done.

http://codereview.chromium.org/6524006/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/6524006/diff/1/src/objects.h#newcode4390
src/objects.h:4390: public:
On 2011/02/15 08:09:31, Mads Ager wrote:
> I would rather make Builtins a friend or make them all public. 

Done.

http://codereview.chromium.org/6524006/diff/4001/src/objects.h#newcode4391
src/objects.h:4391: // Used from builtins-<arch>.cc
Generate_Function(Call/Apply).
Turns out similar trick for byte-wide operations is used for kConstructionCount.
I followed that for endian-ness and while I was at it, included the smi shifting
here, in the proximity of the comment that explains the 64 bit packing.
The codegen looks quite clean this way.