[ignition] Add a property call bytecode

[ignition] Add a property call bytecode

This is a new bytecode which behaves (for now) exactly like Call,
except that in turbofan graph building we can set the
ConvertReceiverMode to NotNullOrUndefined.

I observe a 1% improvement on Box2D, I'd expect a similar improvement on
other OOP heavy code.

Committed: https://crrev.com/c4d770b182f414c52fecaea389dff17c1c97935c
Cr-Commit-Position: refs/heads/master@{#40610}

[Leszek Swirski, 2016-10-26 13:37:20]

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

[commit-bot: I haz the power, 2016-10-26 13:37:29]

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, 2016-10-26 13:53:19]

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

[commit-bot: I haz the power, 2016-10-26 13:53:19]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)

[Leszek Swirski, 2016-10-26 14:27:06]

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

[commit-bot: I haz the power, 2016-10-26 14:27:17]

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

[Leszek Swirski, 2016-10-26 14:42:49]

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

[Leszek Swirski, 2016-10-26 14:42:49]

rmcilroy@chromium.org: Please review changes in interpreter and tests

mstarzinger@chromium.org: Please review (probably just rubber stamp) changes in
bytecode_graph_builder

[commit-bot: I haz the power, 2016-10-26 14:51:28]

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

[commit-bot: I haz the power, 2016-10-26 14:51:29]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2016-10-26 16:20:16]

LGTM with two comments.

https://codereview.chromium.org/2450243002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-builder.cc (right):

https://codereview.chromium.org/2450243002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-array-builder.cc:837: DCHECK(tail_call_mode ==
TailCallMode::kAllow);
We should we do this for tail-calls too? Michi - any idea what is the status of
shipping TCO is?

https://codereview.chromium.org/2450243002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-builder.h (right):

https://codereview.chromium.org/2450243002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-array-builder.h:195: Call::CallType call_type =
Call::OTHER_CALL);
I don't think we should have a default value for this, just always pass it
explicitly.

https://codereview.chromium.org/2450243002/diff/20001/src/interpreter/interpr...
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/2450243002/diff/20001/src/interpreter/interpr...
src/interpreter/interpreter.cc:1633: // into |feedback_slot_id|. The callable is
known to be a named property of the
drop "named"

[Leszek Swirski, 2016-10-26 17:23:49]

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

[commit-bot: I haz the power, 2016-10-26 17:23:51]

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, 2016-10-26 17:52:13]

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

[commit-bot: I haz the power, 2016-10-26 17:52:14]

Dry run: This issue passed the CQ dry run.

[Michael Starzinger, 2016-10-26 17:57:02]

LGTM on "compiler" with one comment. Didn't look at the rest.

https://codereview.chromium.org/2450243002/diff/20001/src/compiler/bytecode-g...
File src/compiler/bytecode-graph-builder.h (right):

https://codereview.chromium.org/2450243002/diff/20001/src/compiler/bytecode-g...
src/compiler/bytecode-graph-builder.h:149: ConvertReceiverMode receiver_hint =
ConvertReceiverMode::kAny);
nit: I think with two call-sites it is better to not have a default argument.

[Michael Starzinger, 2016-10-26 18:03:01]

https://codereview.chromium.org/2450243002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-builder.cc (right):

https://codereview.chromium.org/2450243002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-array-builder.cc:837: DCHECK(tail_call_mode ==
TailCallMode::kAllow);
On 2016/10/26 16:20:16, rmcilroy wrote:
> We should we do this for tail-calls too? Michi - any idea what is the status
of
> shipping TCO is?

It is currently staged as an experimental feature. Shipping plans are unclear. I
don't have strong opinions about whether to add a "TailCallProperty" bytecode or
not, either way is fine with me.

[Leszek Swirski, 2016-10-27 09:08:24]

https://codereview.chromium.org/2450243002/diff/20001/src/compiler/bytecode-g...
File src/compiler/bytecode-graph-builder.h (right):

https://codereview.chromium.org/2450243002/diff/20001/src/compiler/bytecode-g...
src/compiler/bytecode-graph-builder.h:149: ConvertReceiverMode receiver_hint =
ConvertReceiverMode::kAny);
On 2016/10/26 17:57:02, Michael Starzinger wrote:
> nit: I think with two call-sites it is better to not have a default argument.

Done.

https://codereview.chromium.org/2450243002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-builder.cc (right):

https://codereview.chromium.org/2450243002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-array-builder.cc:837: DCHECK(tail_call_mode ==
TailCallMode::kAllow);
On 2016/10/26 16:20:16, rmcilroy wrote:
> We should we do this for tail-calls too? Michi - any idea what is the status
of
> shipping TCO is?

I left it as-is to not have to introduce another bytecode -- I'd be surprised if
we often had tail-calls for property calls, but I may be proven wrong by, e.g.,
callback style programming

https://codereview.chromium.org/2450243002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-builder.h (right):

https://codereview.chromium.org/2450243002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-array-builder.h:195: Call::CallType call_type =
Call::OTHER_CALL);
On 2016/10/26 16:20:16, rmcilroy wrote:
> I don't think we should have a default value for this, just always pass it
> explicitly.

Done.

https://codereview.chromium.org/2450243002/diff/20001/src/interpreter/interpr...
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/2450243002/diff/20001/src/interpreter/interpr...
src/interpreter/interpreter.cc:1633: // into |feedback_slot_id|. The callable is
known to be a named property of the
On 2016/10/26 16:20:16, rmcilroy wrote:
> drop "named"

Done.

[Leszek Swirski, 2016-10-27 09:08:48]

The CQ bit was checked by leszeks@chromium.org

[Leszek Swirski, 2016-10-27 09:08:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
rmcilroy@chromium.org, mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/2450243002/#ps60001
(title: "Address remaining comments")

[commit-bot: I haz the power, 2016-10-27 09:08:57]

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, 2016-10-27 09:35:43]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-17 22:15:00]

Description was changed from

==========
[ignition] Add a property call bytecode

This is a new bytecode which behaves (for now) exactly like Call,
except that in turbofan graph building we can set the
ConvertReceiverMode to NotNullOrUndefined.

I observe a 1% improvement on Box2D, I'd expect a similar improvement on
other OOP heavy code.
==========

to

==========
[ignition] Add a property call bytecode

This is a new bytecode which behaves (for now) exactly like Call,
except that in turbofan graph building we can set the
ConvertReceiverMode to NotNullOrUndefined.

I observe a 1% improvement on Box2D, I'd expect a similar improvement on
other OOP heavy code.

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

[commit-bot: I haz the power, 2016-11-17 22:15:01]

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