[runtime] Remove useless IN builtin.

[runtime] Remove useless IN builtin.

Similar to DELETE, the IN builtin is just a thin wrapper for %HasElement
and %HasProperty anyway, and cannot be optimized, plus it had a weird
special fast case (which also involved at least one LOAD_IC plus some
intrinsic magic).

R=yangguo@chromium.org,jarin@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_win_nosnap_shared_rel

Committed: https://crrev.com/72d60a1e80e81e2e68ca402665e2acbc46c5e471
Cr-Commit-Position: refs/heads/master@{#30154}

Committed: https://crrev.com/3dc9b122fa9f7d551e05f5b9d7415bcfdcac92db
Cr-Commit-Position: refs/heads/master@{#30582}

[Benedikt Meurer, 2015-08-13 12:00:13]

bmeurer@chromium.org changed reviewers:
+ jarin@chromium.org

[Benedikt Meurer, 2015-08-13 12:00:48]

Hey Yang, Jarin,

Another useless builtin gone. Yang, please review the CL overall. Jarin please
take a look at the turbofan part.

Thanks,
Benedikt

[Jarin, 2015-08-13 12:05:56]

lgtm

[Yang, 2015-08-13 12:07:37]

LGTM if removed IN from builtins.h

https://codereview.chromium.org/1295433002/diff/1/src/runtime/runtime-object.cc
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/1295433002/diff/1/src/runtime/runtime-object....
src/runtime/runtime-object.cc:811: Maybe<bool> maybe =
JSReceiver::HasProperty(receiver, name);
This code is duplicate to v8::Object::Has. Maybe we can unify that if you want.

[Benedikt Meurer, 2015-08-13 12:11:13]

https://codereview.chromium.org/1295433002/diff/1/src/runtime/runtime-object.cc
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/1295433002/diff/1/src/runtime/runtime-object....
src/runtime/runtime-object.cc:811: Maybe<bool> maybe =
JSReceiver::HasProperty(receiver, name);
Mhm, yeah, there's moar code duplication with api.cc. We should have a separate
fixit for that.

[Benedikt Meurer, 2015-08-13 12:11:18]

The CQ bit was checked by bmeurer@chromium.org

[Benedikt Meurer, 2015-08-13 12:11:18]

The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org,
yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/1295433002/#ps20001
(title: "Remove IN from builtins.h")

[commit-bot: I haz the power, 2015-08-13 12:11:25]

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

[commit-bot: I haz the power, 2015-08-13 12:39:08]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-08-13 12:39:32]

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

[Benedikt Meurer, 2015-08-13 13:06:29]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1288923002/ by bmeurer@chromium.org.

The reason for reverting is: Breaks win32 nosnap.

[adamk, 2015-08-13 18:04:49]

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

[adamk, 2015-08-13 18:04:50]

The CL description says this builtin is "useless", but the fast path for arrays
was added as a fix for a performance bug:
https://code.google.com/p/v8/issues/detail?id=3701

If this removal is part of a broader effort to improve the handling of 'in',
that's probably fine. Just wanted to call out that this fast path is there for a
reason.

[Benedikt Meurer, 2015-08-14 05:06:34]

On 2015/08/13 18:04:50, adamk wrote:
> The CL description says this builtin is "useless", but the fast path for
arrays
> was added as a fix for a performance bug:
> https://code.google.com/p/v8/issues/detail?id=3701
> 
> If this removal is part of a broader effort to improve the handling of 'in',
> that's probably fine. Just wanted to call out that this fast path is there for
a
> reason.

Thanks a lot for the pointer. I kept the fast path in the runtime, and will
measure with
the microbenchmark whether it's worth keeping a fast path that bypasses the
runtime.

[Benedikt Meurer, 2015-08-14 06:34:22]

Patchset #4 (id:60001) has been deleted

[Yang, 2015-08-14 06:35:33]

On 2015/08/14 05:06:34, Benedikt Meurer wrote:
> On 2015/08/13 18:04:50, adamk wrote:
> > The CL description says this builtin is "useless", but the fast path for
> arrays
> > was added as a fix for a performance bug:
> > https://code.google.com/p/v8/issues/detail?id=3701
> > 
> > If this removal is part of a broader effort to improve the handling of 'in',
> > that's probably fine. Just wanted to call out that this fast path is there
for
> a
> > reason.
> 
> Thanks a lot for the pointer. I kept the fast path in the runtime, and will
> measure with
> the microbenchmark whether it's worth keeping a fast path that bypasses the
> runtime.

lgtm.

[Benedikt Meurer, 2015-08-21 11:31:11]

Patchset #2 (id:20001) has been deleted

[Benedikt Meurer, 2015-08-21 11:31:19]

Patchset #2 (id:40001) has been deleted

[Benedikt Meurer, 2015-08-21 11:31:30]

Patchset #4 (id:120001) has been deleted

[Benedikt Meurer, 2015-09-04 08:20:53]

Patchset #2 (id:80001) has been deleted

[Benedikt Meurer, 2015-09-04 08:21:22]

The CQ bit was checked by bmeurer@chromium.org

[Benedikt Meurer, 2015-09-04 08:21:23]

The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org,
yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/1295433002/#ps160001
(title: "REBASE again. Remove unused ReplaceWithBuiltinCall.")

[commit-bot: I haz the power, 2015-09-04 08:21:35]

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

[commit-bot: I haz the power, 2015-09-04 08:44:30]

Committed patchset #4 (id:160001)

[commit-bot: I haz the power, 2015-09-04 08:44:45]

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