[turbofan] Properly optimize instanceof (even in the presence of @@hasInstance).

[turbofan] Properly optimize instanceof (even in the presence of @@hasInstance).

This is the TurboFan counterpart of http://crrev.com/2504263004, but it
is a bit more involved, since in TurboFan we always inline the appropriate
call to the @@hasInstance handler, and by that we can optimize a lot more
patterns of instanceof than Crankshaft, and even yield fast instanceof
for custom @@hasInstance handlers (which we can now properly inline as
well).

Also we now properly optimize Function.prototype[@@hasInstance], even if
the right hand side of an instanceof doesn't have the Function.prototype
as its direct prototype.

For the baseline case, we still rely on the global protector cell, but
we can address that in a follow-up as well, and make it more robust in
general.

TEST=mjsunit/compiler/instanceof
BUG=v8:5640
R=yangguo@chromium.org

Committed: https://crrev.com/241c024c10a8c1b5d7309299ef61f887363b00a0
Cr-Commit-Position: refs/heads/master@{#41092}

[Benedikt Meurer, 2016-11-18 05:52:35]

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

[commit-bot: I haz the power, 2016-11-18 05:52:40]

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

[Benedikt Meurer, 2016-11-18 05:55:08]

Hey Yang,

Here's the appropriate TurboFan CL for the @@hasInstance fix. It is more general
than the Crankshaft version for a couple of reasons, mostly to avoid the fine
red line of great instanceof performance (having learned that people do already
install custom @@hasInstance handlers).
Please take a look.

Thanks,
Benedikt

Franzi: FYI

[commit-bot: I haz the power, 2016-11-18 06:21:40]

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

[commit-bot: I haz the power, 2016-11-18 06:21:40]

Dry run: This issue passed the CQ dry run.

[Yang, 2016-11-18 06:26:23]

On 2016/11/18 05:55:08, Benedikt Meurer wrote:
> Hey Yang,
> 
> Here's the appropriate TurboFan CL for the @@hasInstance fix. It is more
general
> than the Crankshaft version for a couple of reasons, mostly to avoid the fine
> red line of great instanceof performance (having learned that people do
already
> install custom @@hasInstance handlers).
> Please take a look.
> 
> Thanks,
> Benedikt
> 
> Franzi: FYI

lgtm if comments are addressed.

[Yang, 2016-11-18 06:26:47]

https://codereview.chromium.org/2511223003/diff/1/src/compiler/js-native-cont...
File src/compiler/js-native-context-specialization.cc (right):

https://codereview.chromium.org/2511223003/diff/1/src/compiler/js-native-cont...
src/compiler/js-native-context-specialization.cc:135:
NodeProperties::ChangeOp(node, javascript()->OrdinaryHasInstance());
Would it make sense to constant fold if the lhs is also constant?

https://codereview.chromium.org/2511223003/diff/1/src/compiler/js-native-cont...
src/compiler/js-native-context-specialization.cc:139:
DCHECK(access_info.constant()->IsCallable());
Can we assume this? What if I store a non-callable in receiver.@@hasInstance?

[Benedikt Meurer, 2016-11-18 06:28:51]

https://codereview.chromium.org/2511223003/diff/1/src/compiler/js-native-cont...
File src/compiler/js-native-context-specialization.cc (right):

https://codereview.chromium.org/2511223003/diff/1/src/compiler/js-native-cont...
src/compiler/js-native-context-specialization.cc:135:
NodeProperties::ChangeOp(node, javascript()->OrdinaryHasInstance());
Doable, but as discussed offline, there's no real usecase for this currently.

https://codereview.chromium.org/2511223003/diff/1/src/compiler/js-native-cont...
src/compiler/js-native-context-specialization.cc:139:
DCHECK(access_info.constant()->IsCallable());
Yes, all data constants stored on the map are currently JSFunctions, so at the
very least they are callable.

[Benedikt Meurer, 2016-11-18 06:28:56]

The CQ bit was checked by bmeurer@chromium.org

[commit-bot: I haz the power, 2016-11-18 06:29:01]

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-11-18 06:31:04]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-18 06:31:53]

Description was changed from

==========
[turbofan] Properly optimize instanceof (even in the presence of @@hasInstance).

This is the TurboFan counterpart of http://crrev.com/2504263004, but it
is a bit more involved, since in TurboFan we always inline the appropriate
call to the @@hasInstance handler, and by that we can optimize a lot more
patterns of instanceof than Crankshaft, and even yield fast instanceof
for custom @@hasInstance handlers (which we can now properly inline as
well).

Also we now properly optimize Function.prototype[@@hasInstance], even if
the right hand side of an instanceof doesn't have the Function.prototype
as its direct prototype.

For the baseline case, we still rely on the global protector cell, but
we can address that in a follow-up as well, and make it more robust in
general.

TEST=mjsunit/compiler/instanceof
BUG=v8:5640
R=yangguo@chromium.org
==========

to

==========
[turbofan] Properly optimize instanceof (even in the presence of @@hasInstance).

This is the TurboFan counterpart of http://crrev.com/2504263004, but it
is a bit more involved, since in TurboFan we always inline the appropriate
call to the @@hasInstance handler, and by that we can optimize a lot more
patterns of instanceof than Crankshaft, and even yield fast instanceof
for custom @@hasInstance handlers (which we can now properly inline as
well).

Also we now properly optimize Function.prototype[@@hasInstance], even if
the right hand side of an instanceof doesn't have the Function.prototype
as its direct prototype.

For the baseline case, we still rely on the global protector cell, but
we can address that in a follow-up as well, and make it more robust in
general.

TEST=mjsunit/compiler/instanceof
BUG=v8:5640
R=yangguo@chromium.org

Committed: https://crrev.com/241c024c10a8c1b5d7309299ef61f887363b00a0
Cr-Commit-Position: refs/heads/master@{#41092}
==========

[commit-bot: I haz the power, 2016-11-18 06:31:54]

Patchset 1 (id:??) landed as
https://crrev.com/241c024c10a8c1b5d7309299ef61f887363b00a0
Cr-Commit-Position: refs/heads/master@{#41092}