[ignition] Only initialize [[HomeObject]] for class constructors if needed

[ignition] Only initialize [[HomeObject]] for class constructors if needed

This moves the initialization of [[HomeObject]] for constructors from
the %DefineClass runtime function into the bytecode generator, and
makes it conditional (resolving an old TODO). As part of this refactor,
avoid a load of "prototype" by returning the class prototype from
%DefineClass.

This is one of many steps in moving more of class definition into
bytecode.

R=rmcilroy@chromium.org

Review-Url: https://codereview.chromium.org/2610683003
Cr-Commit-Position: refs/heads/master@{#42072}
Committed: https://chromium.googlesource.com/v8/v8/+/3e20d381eddec3198ef0de790f099bd0c08bbb33

[adamk, 2017-01-04 02:25:48]

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

[commit-bot: I haz the power, 2017-01-04 02:25:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-04 03:15:24]

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

[commit-bot: I haz the power, 2017-01-04 03:15:24]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2017-01-04 09:55:59]

Awesome, LGTM, thanks!

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

https://codereview.chromium.org/2610683003/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.cc:1415:
VisitForRegisterValue(expr->constructor(), constructor);
nit - you can just do:
Register constructor = VisitForRegisterValue(expr->constructor());

https://codereview.chromium.org/2610683003/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.cc:1416: Register prototype =
register_allocator()->NewRegister();
nit - you could save a register if you allocate this after the runtime call
block and move the StoreAccumulatorInRegister(prototype) out of that block.

https://codereview.chromium.org/2610683003/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.cc:1433: // prototype is already in the
accumulator
nit - fullstop and capital letter for comments.

[adamk, 2017-01-04 17:56:57]

The CQ bit was checked by adamk@chromium.org

[adamk, 2017-01-04 17:56:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/2610683003/#ps20001
(title: "Review comments")

[commit-bot: I haz the power, 2017-01-04 17:57:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[adamk, 2017-01-04 17:57:04]

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

https://codereview.chromium.org/2610683003/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.cc:1415:
VisitForRegisterValue(expr->constructor(), constructor);
On 2017/01/04 09:55:59, rmcilroy wrote:
> nit - you can just do:
> Register constructor = VisitForRegisterValue(expr->constructor());

Ah, nice.

https://codereview.chromium.org/2610683003/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.cc:1416: Register prototype =
register_allocator()->NewRegister();
On 2017/01/04 09:55:59, rmcilroy wrote:
> nit - you could save a register if you allocate this after the runtime call
> block and move the StoreAccumulatorInRegister(prototype) out of that block.

Done.

https://codereview.chromium.org/2610683003/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.cc:1433: // prototype is already in the
accumulator
On 2017/01/04 09:55:59, rmcilroy wrote:
> nit - fullstop and capital letter for comments.

Done.

[commit-bot: I haz the power, 2017-01-04 18:10:49]

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

[commit-bot: I haz the power, 2017-01-04 18:10:50]

Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux64_gyp_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng_trigg...)

[adamk, 2017-01-04 18:51:34]

The CQ bit was checked by adamk@chromium.org

[adamk, 2017-01-04 18:51:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/2610683003/#ps40001
(title: "Fix bytecode expectations")

[commit-bot: I haz the power, 2017-01-04 18:51:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-04 19:15:27]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1483555894021780,
"parent_rev": "f81590a1c8344cf87e72e312722abe0b35d9601f", "commit_rev":
"3e20d381eddec3198ef0de790f099bd0c08bbb33"}

[commit-bot: I haz the power, 2017-01-04 19:15:30]

Description was changed from

==========
[ignition] Only initialize [[HomeObject]] for class constructors if needed

This moves the initialization of [[HomeObject]] for constructors from
the %DefineClass runtime function into the bytecode generator, and
makes it conditional (resolving an old TODO). As part of this refactor,
avoid a load of "prototype" by returning the class prototype from
%DefineClass.

This is one of many steps in moving more of class definition into
bytecode.

R=rmcilroy@chromium.org
==========

to

==========
[ignition] Only initialize [[HomeObject]] for class constructors if needed

This moves the initialization of [[HomeObject]] for constructors from
the %DefineClass runtime function into the bytecode generator, and
makes it conditional (resolving an old TODO). As part of this refactor,
avoid a load of "prototype" by returning the class prototype from
%DefineClass.

This is one of many steps in moving more of class definition into
bytecode.

R=rmcilroy@chromium.org

Review-Url: https://codereview.chromium.org/2610683003
Cr-Commit-Position: refs/heads/master@{#42072}
Committed:
https://chromium.googlesource.com/v8/v8/+/3e20d381eddec3198ef0de790f099bd0c08...
==========

[commit-bot: I haz the power, 2017-01-04 19:15:31]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/3e20d381eddec3198ef0de790f099bd0c08...

[arv (Not doing code reviews), 2017-01-05 05:18:41]

arv@chromium.org changed reviewers:
+ arv@chromium.org

[arv (Not doing code reviews), 2017-01-05 05:18:43]

https://codereview.chromium.org/2610683003/diff/40001/src/runtime/runtime-cla...
File src/runtime/runtime-classes.cc (left):

https://codereview.chromium.org/2610683003/diff/40001/src/runtime/runtime-cla...
src/runtime/runtime-classes.cc:170: // TODO(arv): Only do this conditionally.
Thanks