[Interpreter] Fill out function prologue support.

[Interpreter] Fill out function prologue support.

Fills out some more of the function prologue support in the
interpreter. Deals with creation of arguments objects and throwing
IllegalRedeclarations if necessary. Also adds (untested) support for
this.function and new.target variable assignment.

Also fixes a bug in Frames::is_java_script() to deal with
interpreter frames correctly.

Cleans up comments in builtins InterpreterEntryTrampoline about
missing prologue support.

Adds the following bytecodes:
  - CreateArgumentsSloppy
  - CreateArgumentsStrict

BUG=v8:4280
LOG=N

Committed: https://crrev.com/6256e1dcd5c45678b7fd03f7810283a303fd0ef5
Cr-Commit-Position: refs/heads/master@{#31486}

[rmcilroy, 2015-10-21 08:56:21]

Description was changed from

==========
[Interpreter] Fill out function prologue support.

Fills out some more of the function prologue support in the
interpreter. Deals with creation of arguments objects and throwing
IllegalRedeclarations if necessary. Also adds (untested) support for
this.function and new.target variable assignment.

Cleans up comments in builtins InterpreterEntryTrampoline about
missing prologue support.

Adds the following bytecodes:
  - CreateArgumentsSloppy
  - CreateArgumentsStrict

BUG=v8:4280
LOG=N
==========

to

==========
[Interpreter] Fill out function prologue support.

Fills out some more of the function prologue support in the
interpreter. Deals with creation of arguments objects and throwing
IllegalRedeclarations if necessary. Also adds (untested) support for
this.function and new.target variable assignment.

Also fixes a bug in Frames::is_java_script() to deal with
interpreter frames correctly.

Cleans up comments in builtins InterpreterEntryTrampoline about
missing prologue support.

Adds the following bytecodes:
  - CreateArgumentsSloppy
  - CreateArgumentsStrict

BUG=v8:4280
LOG=N
==========

[rmcilroy, 2015-10-21 08:57:12]

rmcilroy@chromium.org changed reviewers:
+ bmeurer@chromium.org, oth@chromium.org

[rmcilroy, 2015-10-21 08:57:13]

Benedikt, Orion, PTAL, thanks.

[rmcilroy, 2015-10-21 10:20:36]

rmcilroy@chromium.org changed reviewers:
+ mstarzinger@chromium.org

[rmcilroy, 2015-10-21 10:20:37]

+Michi

[oth, 2015-10-21 15:22:41]

lgtm

[Michael Starzinger, 2015-10-21 18:42:18]

LGTM.

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecode-ar...
File src/interpreter/bytecode-array-builder.h (right):

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-builder.h:100: // Create a new arguments object
in the accumulator, calling the |function_id|
nit: Comment referring to "function_id" looks outdated.

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecode-ge...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.cc:1406: if (arguments == NULL) return;
nit: s/NULL/nullptr/

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecode-ge...
File src/interpreter/bytecode-generator.h (right):

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.h:51: void VisitNewTargetVariable(Variable*
this_function_var);
nit: s/this_function_var/new_target_var/. Or even better, just pick "variable"
for all of them to avoid this and have them all be uniform.

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecodes.h
File src/interpreter/bytecodes.h (right):

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecodes.h...
src/interpreter/bytecodes.h:125: V(CreateArgumentsSloppy, OperandType::kNone)   
                             \
suggestion: I know we are inconsistent with the naming, in ES5 the
differentiation between those two "arguments objects" was purely done on
language mode and these names made sense. But the ES6 spec refers to them as
"mapped arguments object"[1] and "unmapped arguments object"[2] and they are
chosen not only depending on language mode. In TurboFan e.g. we started using
the new naming scheme (see CreateArgumentsParameters::Type) recently. So I would
vote for calling these bytecodes "CreateArgumentsMapped" and
"CreateArgumentsUnmapped". Just a suggestion, no strong opinion, ultimately it's
up to you. :)

[1]
http://www.ecma-international.org/ecma-262/6.0/#sec-createmappedargumentsobject
[2]
http://www.ecma-international.org/ecma-262/6.0/#sec-createunmappedargumentsob...

[Michael Starzinger, 2015-10-21 18:44:37]

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/interpreter.cc
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/interpreter...
src/interpreter/interpreter.cc:911: // CreateArgumentsSloppy <type>
nit: Comment looks outdated, no <type> parameter.

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/interpreter...
src/interpreter/interpreter.cc:923: // CreateArgumentsStrict <type>
nit: Likewise.

[Benedikt Meurer, 2015-10-22 04:57:09]

lgtm

[rmcilroy, 2015-10-22 15:19:52]

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecode-ar...
File src/interpreter/bytecode-array-builder.h (right):

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-builder.h:100: // Create a new arguments object
in the accumulator, calling the |function_id|
On 2015/10/21 18:42:17, Michael Starzinger wrote:
> nit: Comment referring to "function_id" looks outdated.

Done.

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecode-ge...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.cc:1406: if (arguments == NULL) return;
On 2015/10/21 18:42:17, Michael Starzinger wrote:
> nit: s/NULL/nullptr/

Done.

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecode-ge...
File src/interpreter/bytecode-generator.h (right):

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.h:51: void VisitNewTargetVariable(Variable*
this_function_var);
On 2015/10/21 18:42:17, Michael Starzinger wrote:
> nit: s/this_function_var/new_target_var/. Or even better, just pick "variable"
> for all of them to avoid this and have them all be uniform.

Done.

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecodes.h
File src/interpreter/bytecodes.h (right):

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/bytecodes.h...
src/interpreter/bytecodes.h:125: V(CreateArgumentsSloppy, OperandType::kNone)   
                             \
On 2015/10/21 18:42:17, Michael Starzinger wrote:
> suggestion: I know we are inconsistent with the naming, in ES5 the
> differentiation between those two "arguments objects" was purely done on
> language mode and these names made sense. But the ES6 spec refers to them as
> "mapped arguments object"[1] and "unmapped arguments object"[2] and they are
> chosen not only depending on language mode. In TurboFan e.g. we started using
> the new naming scheme (see CreateArgumentsParameters::Type) recently. So I
would
> vote for calling these bytecodes "CreateArgumentsMapped" and
> "CreateArgumentsUnmapped". Just a suggestion, no strong opinion, ultimately
it's
> up to you. :)
> 
> [1]
>
http://www.ecma-international.org/ecma-262/6.0/#sec-createmappedargumentsobject
> [2]
>
http://www.ecma-international.org/ecma-262/6.0/#sec-createunmappedargumentsob...

Good call, went with CreateUnmappedArguments and CreateMappedArguments.

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/interpreter.cc
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/interpreter...
src/interpreter/interpreter.cc:911: // CreateArgumentsSloppy <type>
On 2015/10/21 18:44:37, Michael Starzinger wrote:
> nit: Comment looks outdated, no <type> parameter.

Done.

https://codereview.chromium.org/1412953007/diff/1/src/interpreter/interpreter...
src/interpreter/interpreter.cc:923: // CreateArgumentsStrict <type>
On 2015/10/21 18:44:37, Michael Starzinger wrote:
> nit: Likewise.

Done.

[rmcilroy, 2015-10-22 21:04:39]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2015-10-22 21:04:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, oth@chromium.org, mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1412953007/#ps40001
(title: "Rebased")

[commit-bot: I haz the power, 2015-10-22 21:04:43]

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

[commit-bot: I haz the power, 2015-10-22 21:41:55]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-22 21:42:14]

Patchset 3 (id:??) landed as
https://crrev.com/6256e1dcd5c45678b7fd03f7810283a303fd0ef5
Cr-Commit-Position: refs/heads/master@{#31486}