[turbofan] When inlining JSCallConstruct receiver should be set to the hole.

[turbofan] When inlining JSCallConstruct receiver should be set to the hole.

When inlining JSCallConstruct in turbofan, receiver is initialized to model
the behaviour of constructor. When an implicit receiver is not required the
receiver value should be set to the hole value instead of undefined value.
When initializing the receiver via super calls, we check that the receiver
is the hole value.

BUG=chromium:653407
Committed: https://crrev.com/cad36659b1dc230773eb6d2a985fa8b9a12a4d86
Cr-Commit-Position: refs/heads/master@{#40396}

[mythria, 2016-10-17 18:28:25]

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

[commit-bot: I haz the power, 2016-10-17 18:28:31]

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-17 18:55:05]

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

[commit-bot: I haz the power, 2016-10-17 18:55:07]

Dry run: This issue passed the CQ dry run.

[mythria, 2016-10-17 18:56:23]

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

[mythria, 2016-10-17 18:56:23]

Hi Michi,

This is to fix this bug:
https://bugs.chromium.org/p/chromium/issues/detail?id=653407. 

If my understanding is correct when inlining JSCallConstruct, the receiver
should be initialized to the hole value  as done in
Generate_JSConstructStubHelper
(https://cs.chromium.org/chromium/src/v8/src/builtins/x64/builtins-x64.cc?dr=C...).


I think this bug affects only Ignition+Turbofan because in Turbofan, we
eliminate adding hole checks when it is known that the "this" value is known to
be a hole constant. I guess we can do something similar in
bytecode-graph-builder as well for performance but should not affect
correctness.  Let me know if you think this is the right fix. 

Thanks and regards,
Mythri

[mythria, 2016-10-17 18:57:39]

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

[commit-bot: I haz the power, 2016-10-17 18:57:46]

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-17 19:16:07]

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

[commit-bot: I haz the power, 2016-10-17 19:16:08]

Dry run: This issue passed the CQ dry run.

[Michael Starzinger, 2016-10-18 09:39:41]

LGTM.

Can we add a regression test for this? Preferably one without a loop, but using
%OptimizeFunctionOnNextCall instead.

Yeah, I think this is BytecodeGraphBuilder only because for the AstGraphBuilder
it is probably always hidden by the fake TDZ initialization we had to put into
the graph:

https://cs.chromium.org/chromium/src/v8/src/compiler/ast-graph-builder.cc?q=A...

[mythria, 2016-10-18 10:41:16]

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

[commit-bot: I haz the power, 2016-10-18 10:41:24]

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

[mythria, 2016-10-18 10:43:50]

Thanks Michi. I added a test without the loop. I will land this once it passes
the bots.

[commit-bot: I haz the power, 2016-10-18 11:03:33]

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

[commit-bot: I haz the power, 2016-10-18 11:03:34]

Dry run: This issue passed the CQ dry run.

[mythria, 2016-10-18 11:41:51]

The CQ bit was checked by mythria@chromium.org

[mythria, 2016-10-18 11:41:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/2424123002/#ps20001
(title: "Added a regress test.")

[commit-bot: I haz the power, 2016-10-18 11:42:02]

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-18 11:47:45]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-18 11:48:24]

Description was changed from

==========
[turbofan] When inlining JSCallConstruct receiver should be set to the hole.

When inlining JSCallConstruct in turbofan, receiver is initialized to model
the behaviour of constructor. When an implicit receiver is not required the
receiver value should be set to the hole value instead of undefined value.
When initializing the receiver via super calls, we check that the receiver
is the hole value.

BUG=chromium:653407
==========

to

==========
[turbofan] When inlining JSCallConstruct receiver should be set to the hole.

When inlining JSCallConstruct in turbofan, receiver is initialized to model
the behaviour of constructor. When an implicit receiver is not required the
receiver value should be set to the hole value instead of undefined value.
When initializing the receiver via super calls, we check that the receiver
is the hole value.

BUG=chromium:653407

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

[commit-bot: I haz the power, 2016-10-18 11:48:25]

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