Ensure correct boxing of values when calling functions on them...

Ensure correct boxing of values when calling functions on them

When a function is called with a value type as the receiver this is now boxed as an object.

This is a low-impact solution where the receiver is only boxed when required. For IC calls to the V8 builtins values are not boxed and as most of the functions on String.prototype, Number.prototype and Boolean.prototype are sitting there most IC calls on values will not need any boxing of the receiver.

For calls which are not IC calls but calls through the CallFunctionStub a flag is used to determine whether the receiver might be a value and only when that is the case will the receiver be boxed.

No changtes to Function.call and Function.apply - they already boxed values. According to the ES5 spec the receiver should not be boxed for these functions, but current browsers have not adopted that change yet.

BUG=223
TEST=test/mjsunit/value-wrapper.js
TEST=test/mjsunit/regress/regress-crbug-3184.js

Committed: http://code.google.com/p/v8/source/detail?r=3617

[Mads Ager (chromium), 2010-01-15 12:15:56]

http://codereview.chromium.org/542087/diff/3011/4023
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/542087/diff/3011/4023#newcode640
src/arm/stub-cache-arm.cc:640: if (!function->IsBuiltin()) {
Could you add a comment for these cases stating that for builtins we don't need
to wrap the receiver, but for other functions we have to wrap the receiver and
therefore enter the runtime system.  Otherwise, I think these conditionals will
be hard to understand later on.

http://codereview.chromium.org/542087/diff/3011/4017
File src/codegen.h (right):

http://codereview.chromium.org/542087/diff/3011/4017#newcode540
src/codegen.h:540: // Minor key encoding in 31 bits
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAARL.
RL -> FI for F(lag)I(nloop) ?

http://codereview.chromium.org/542087/diff/3011/4012
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/542087/diff/3011/4012#newcode8704
src/ia32/codegen-ia32.cc:8704: __ InvokeBuiltin(Builtins::TO_OBJECT,
CALL_FUNCTION);
Don't we need a frame here as in the ARM version to let the GC know about the
return address that is pushed on the stack?  This stub is called from JavaScript
code which might move on compacting collections.

http://codereview.chromium.org/542087/diff/3011/4014
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/542087/diff/3011/4014#newcode1000
src/ia32/stub-cache-ia32.cc:1000: if (!function->IsBuiltin()) {
Add comments on these.

http://codereview.chromium.org/542087/diff/3011/4021
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/542087/diff/3011/4021#newcode6990
src/x64/codegen-x64.cc:6990: __ InvokeBuiltin(Builtins::TO_OBJECT,
CALL_FUNCTION);
Don't we need a frame here as in the ARM version to let the GC know about the
return address that is pushed on the stack?  This stub is called from JavaScript
code which might move on compacting collections.

http://codereview.chromium.org/542087/diff/3011/4019
File src/x64/stub-cache-x64.cc (right):

http://codereview.chromium.org/542087/diff/3011/4019#newcode724
src/x64/stub-cache-x64.cc:724: if (!function->IsBuiltin()) {
Comments.

[Søren Thygesen Gjesse, 2010-01-15 14:49:49]

http://codereview.chromium.org/542087/diff/3011/4023
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/542087/diff/3011/4023#newcode640
src/arm/stub-cache-arm.cc:640: if (!function->IsBuiltin()) {
On 2010/01/15 12:15:56, Mads Ager wrote:
> Could you add a comment for these cases stating that for builtins we don't
need
> to wrap the receiver, but for other functions we have to wrap the receiver and
> therefore enter the runtime system.  Otherwise, I think these conditionals
will
> be hard to understand later on.

Done.

http://codereview.chromium.org/542087/diff/3011/4017
File src/codegen.h (right):

http://codereview.chromium.org/542087/diff/3011/4017#newcode540
src/codegen.h:540: // Minor key encoding in 31 bits
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAARL.
On 2010/01/15 12:15:56, Mads Ager wrote:
> RL -> FI for F(lag)I(nloop) ?

Done.

http://codereview.chromium.org/542087/diff/3011/4012
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/542087/diff/3011/4012#newcode8704
src/ia32/codegen-ia32.cc:8704: __ InvokeBuiltin(Builtins::TO_OBJECT,
CALL_FUNCTION);
On 2010/01/15 12:15:56, Mads Ager wrote:
> Don't we need a frame here as in the ARM version to let the GC know about the
> return address that is pushed on the stack?  This stub is called from
JavaScript
> code which might move on compacting collections.

Good catch, we sure we do!

http://codereview.chromium.org/542087/diff/3011/4014
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/542087/diff/3011/4014#newcode1000
src/ia32/stub-cache-ia32.cc:1000: if (!function->IsBuiltin()) {
On 2010/01/15 12:15:56, Mads Ager wrote:
> Add comments on these.

Done.

http://codereview.chromium.org/542087/diff/3011/4021
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/542087/diff/3011/4021#newcode6990
src/x64/codegen-x64.cc:6990: __ InvokeBuiltin(Builtins::TO_OBJECT,
CALL_FUNCTION);
On 2010/01/15 12:15:56, Mads Ager wrote:
> Don't we need a frame here as in the ARM version to let the GC know about the
> return address that is pushed on the stack?  This stub is called from
JavaScript
> code which might move on compacting collections.

Good catch, we sure we do!

http://codereview.chromium.org/542087/diff/3011/4019
File src/x64/stub-cache-x64.cc (right):

http://codereview.chromium.org/542087/diff/3011/4019#newcode724
src/x64/stub-cache-x64.cc:724: if (!function->IsBuiltin()) {
On 2010/01/15 12:15:56, Mads Ager wrote:
> Comments.

Done.

[Mads Ager (chromium), 2010-01-19 07:28:58]

LGTM!

http://codereview.chromium.org/542087/diff/6028/5015
File test/mjsunit/value-wrapper.js (right):

http://codereview.chromium.org/542087/diff/6028/5015#newcode124
test/mjsunit/value-wrapper.js:124: // According to ES3 15.3.4.3 the this value
passed to Function.prototyle.apply
prototyle -> prototype