[runtime] Fix ES6 9.2.1 [[Call]] when encountering a classConstructor.

[runtime] Fix ES6 9.2.1 [[Call]] when encountering a classConstructor.

The current implementation of classes throws the TypeError at the wrong
point, after activating a new context when directly calling a class
constructor. According to the spec, the TypeError has to be thrown
in the caller context.

LOG=N
BUG=v8:4428
Committed: https://crrev.com/6a06bc0a774933719f62009d81b3f1686d83bb90
Cr-Commit-Position: refs/heads/master@{#31786}

[Camillo Bruni, 2015-10-22 10:49:58]

cbruni@chromium.org changed reviewers:
+ bmeurer@chromium.org

[Camillo Bruni, 2015-10-22 10:49:59]

PTAL, Is this roughly what you intended for the fullcodegen ICStubs?

[Benedikt Meurer, 2015-10-22 11:02:30]

Description was changed from

==========
[runtime] Fix ES6 9.2.1 [[Call]] when encountering a classConstructor
The current implementation of classes throws the TypeError at the wrong
point, after activating a new context when directly calling a class
constructor. According to the spec, the TypeError has to be thrown
in the caller context.

LOG=N
BUG=v8:4428
==========

to

==========
[runtime] Fix ES6 9.2.1 [[Call]] when encountering a classConstructor.

The current implementation of classes throws the TypeError at the wrong
point, after activating a new context when directly calling a class
constructor. According to the spec, the TypeError has to be thrown
in the caller context.

LOG=N
BUG=v8:4428
==========

[Benedikt Meurer, 2015-10-22 11:08:05]

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

https://codereview.chromium.org/1418623007/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:1719:
STATIC_ASSERT(SharedFunctionInfo::kNativeByteOffset ==
Move the STATIC_ASSERT to the actual use site.

https://codereview.chromium.org/1418623007/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:1725: __ movp(rdx, FieldOperand(rdi,
JSFunction::kSharedFunctionInfoOffset));
No need to load the shared function info twice.

https://codereview.chromium.org/1418623007/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:1727: __ testb(FieldOperand(rdx,
SharedFunctionInfo::kIsArrowByteOffset),
kIsArrowByteOffset is very confusing.

https://codereview.chromium.org/1418623007/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:1729: __ j(zero, &non_classConstructor);
You can use Label::kNear here.

https://codereview.chromium.org/1418623007/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:1735: __ bind(&non_classConstructor);
Nit: non_class_constructor

https://codereview.chromium.org/1418623007/diff/1/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):

https://codereview.chromium.org/1418623007/diff/1/src/x64/code-stubs-x64.cc#n...
src/x64/code-stubs-x64.cc:2111: {
You need to do the same in the CallFunctionStub.

https://codereview.chromium.org/1418623007/diff/1/src/x64/code-stubs-x64.cc#n...
src/x64/code-stubs-x64.cc:2117: __ j(zero, &non_classConstructor);
You can use Label::kNear here.

https://codereview.chromium.org/1418623007/diff/1/src/x64/code-stubs-x64.cc#n...
src/x64/code-stubs-x64.cc:2125: __ InvokeFunction(rdi, actual, JUMP_FUNCTION,
NullCallWrapper());
Please check if it's viable to use __Jump(builtin->CallFunction()) here.

[Camillo Bruni, 2015-11-03 10:03:12]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-03 10:03:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1418623007/180002
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1418623007/180002

[commit-bot: I haz the power, 2015-11-03 10:29:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-03 10:29:56]

Dry run: This issue passed the CQ dry run.

[Camillo Bruni, 2015-11-03 16:05:27]

PTAL
For now I didn't merge the CallFunction builtin with the code in the CallIC
stub.

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

https://codereview.chromium.org/1418623007/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:1719:
STATIC_ASSERT(SharedFunctionInfo::kNativeByteOffset ==
On 2015/10/22 at 11:08:05, Benedikt Meurer wrote:
> Move the STATIC_ASSERT to the actual use site.

done

https://codereview.chromium.org/1418623007/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:1725: __ movp(rdx, FieldOperand(rdi,
JSFunction::kSharedFunctionInfoOffset));
On 2015/10/22 at 11:08:04, Benedikt Meurer wrote:
> No need to load the shared function info twice.

done

https://codereview.chromium.org/1418623007/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:1727: __ testb(FieldOperand(rdx,
SharedFunctionInfo::kIsArrowByteOffset),
On 2015/10/22 at 11:08:04, Benedikt Meurer wrote:
> kIsArrowByteOffset is very confusing.

done

https://codereview.chromium.org/1418623007/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:1729: __ j(zero, &non_classConstructor);
On 2015/10/22 at 11:08:04, Benedikt Meurer wrote:
> You can use Label::kNear here.

done

https://codereview.chromium.org/1418623007/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:1735: __ bind(&non_classConstructor);
On 2015/10/22 at 11:08:04, Benedikt Meurer wrote:
> Nit: non_class_constructor

done

https://codereview.chromium.org/1418623007/diff/1/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):

https://codereview.chromium.org/1418623007/diff/1/src/x64/code-stubs-x64.cc#n...
src/x64/code-stubs-x64.cc:2117: __ j(zero, &non_classConstructor);
On 2015/10/22 at 11:08:05, Benedikt Meurer wrote:
> You can use Label::kNear here.

done

[Benedikt Meurer, 2015-11-04 02:56:43]

https://codereview.chromium.org/1418623007/diff/180002/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):

https://codereview.chromium.org/1418623007/diff/180002/src/arm/builtins-arm.c...
src/arm/builtins-arm.cc:1561: STATIC_ASSERT((FunctionKind::kClassConstructor <<
kSmiTagSize) <
Please use the kFooBitWithinByte trick that we use for Native and Strict bits
below, Instead of this STATIC_ASSERT and the adhoc <<kSmiTagSize below.

Same for the other architectures.

https://codereview.chromium.org/1418623007/diff/180002/src/crankshaft/hydroge...
File src/crankshaft/hydrogen.cc (right):

https://codereview.chromium.org/1418623007/diff/180002/src/crankshaft/hydroge...
src/crankshaft/hydrogen.cc:12761: HInstruction* invoke_result =
Add<HInvokeFunction>(function, arg_count);
Nit: unrelated change, please undo

https://codereview.chromium.org/1418623007/diff/180002/src/crankshaft/hydroge...
src/crankshaft/hydrogen.cc:12770: HInstruction* call_result =
Add<HCallFunction>(function, arg_count);
Nit: unrelated change, please undo

[Camillo Bruni, 2015-11-04 10:47:32]

PTAL
Introducing and Using kClassConstructorBitsWithinByte for consistency

https://codereview.chromium.org/1418623007/diff/180002/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):

https://codereview.chromium.org/1418623007/diff/180002/src/arm/builtins-arm.c...
src/arm/builtins-arm.cc:1561: STATIC_ASSERT((FunctionKind::kClassConstructor <<
kSmiTagSize) <
On 2015/11/04 at 02:56:43, Benedikt Meurer wrote:
> Please use the kFooBitWithinByte trick that we use for Native and Strict bits
below, Instead of this STATIC_ASSERT and the adhoc <<kSmiTagSize below.
> 
> Same for the other architectures.

done

https://codereview.chromium.org/1418623007/diff/180002/src/crankshaft/hydroge...
File src/crankshaft/hydrogen.cc (right):

https://codereview.chromium.org/1418623007/diff/180002/src/crankshaft/hydroge...
src/crankshaft/hydrogen.cc:12761: HInstruction* invoke_result =
Add<HInvokeFunction>(function, arg_count);
On 2015/11/04 at 02:56:43, Benedikt Meurer wrote:
> Nit: unrelated change, please undo

undone

https://codereview.chromium.org/1418623007/diff/180002/src/crankshaft/hydroge...
src/crankshaft/hydrogen.cc:12770: HInstruction* call_result =
Add<HCallFunction>(function, arg_count);
On 2015/11/04 at 02:56:43, Benedikt Meurer wrote:
> Nit: unrelated change, please undo

undone

[Benedikt Meurer, 2015-11-04 10:51:49]

Thanks. LGTM.

[Camillo Bruni, 2015-11-04 12:58:51]

The CQ bit was checked by cbruni@chromium.org

[commit-bot: I haz the power, 2015-11-04 12:58:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1418623007/370001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1418623007/370001

[commit-bot: I haz the power, 2015-11-04 13:23:51]

Committed patchset #20 (id:370001)

[commit-bot: I haz the power, 2015-11-04 13:24:22]

Patchset 20 (id:??) landed as
https://crrev.com/6a06bc0a774933719f62009d81b3f1686d83bb90
Cr-Commit-Position: refs/heads/master@{#31786}

[Camillo Bruni, 2015-11-04 13:55:48]

A revert of this CL (patchset #20 id:370001) has been created in
https://codereview.chromium.org/1415783006/ by cbruni@chromium.org.

The reason for reverting is: failing build bot.

[Camillo Bruni, 2015-11-04 14:06:01]

The CQ bit was checked by cbruni@chromium.org

[Camillo Bruni, 2015-11-04 14:06:02]

The patchset sent to the CQ was uploaded after l-g-t-m from bmeurer@chromium.org
Link to the patchset: https://codereview.chromium.org/1418623007/#ps390001
(title: "Do not use kNear jump on x64")

[commit-bot: I haz the power, 2015-11-04 14:06:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1418623007/390001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1418623007/390001

[commit-bot: I haz the power, 2015-11-04 14:30:00]

Committed patchset #21 (id:390001)

[commit-bot: I haz the power, 2015-11-04 14:30:16]

Patchset 21 (id:??) landed as
https://crrev.com/ab84025977fb94bebf7d4d83b874da5a6c1c6293
Cr-Commit-Position: refs/heads/master@{#31790}

[Camillo Bruni, 2015-11-04 15:44:21]

Description was changed from

==========
[runtime] Fix ES6 9.2.1 [[Call]] when encountering a classConstructor.

The current implementation of classes throws the TypeError at the wrong
point, after activating a new context when directly calling a class
constructor. According to the spec, the TypeError has to be thrown
in the caller context.

LOG=N
BUG=v8:4428

Committed: https://crrev.com/6a06bc0a774933719f62009d81b3f1686d83bb90
Cr-Commit-Position: refs/heads/master@{#31786}

Committed: https://crrev.com/ab84025977fb94bebf7d4d83b874da5a6c1c6293
Cr-Commit-Position: refs/heads/master@{#31790}
==========

to

==========
[runtime] Fix ES6 9.2.1 [[Call]] when encountering a classConstructor.

The current implementation of classes throws the TypeError at the wrong
point, after activating a new context when directly calling a class
constructor. According to the spec, the TypeError has to be thrown
in the caller context.

LOG=N
BUG=v8:4428

Committed: https://crrev.com/6a06bc0a774933719f62009d81b3f1686d83bb90
Cr-Commit-Position: refs/heads/master@{#31786}
==========

[Camillo Bruni, 2015-11-05 09:40:14]

Could the MIPS team please check this CL? Thanks :)

[akos.palfi.imgtec, 2015-11-05 10:40:58]

On 2015/11/05 09:40:14, cbruni wrote:
> Could the MIPS team please check this CL? Thanks :)

MIPS part LGTM, thanks!