Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(112)

Issue 2370693002: [compiler] Properly guard the speculative optimizations for instanceof. (Closed)

Created:
4 years, 2 months ago by Benedikt Meurer
Modified:
4 years, 2 months ago
Reviewers:
mvstanton
CC:
v8-reviews_googlegroups.com, v8-mips-ports_googlegroups.com, v8-x87-ports_googlegroups.com, v8-ppc-ports_googlegroups.com, rmcilroy
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[compiler] Properly guard the speculative optimizations for instanceof. Add a general feedback slot for instanceof similar to what we already have for for-in, which basically has a fast (indicated by the uninitialized sentinel) and a slow (indicated by the megamorphic sentinel) mode. Now we can only take the fast path when the feedback slot says it hasn't seen any funky inputs and nothing funky appeared in the prototype chain. In the TurboFan code we also deoptimize whenever we see a funky object (i.e. a proxy or an object that requires access checks) in the prototype chain (similar to what Crankshaft already did). Drive-by-fix: Also make Crankshaft respect the mode and therefore address the deopt loop in Crankshaft around instanceof. We might want to introduce an InstanceOfIC mechanism at some point and track the map of the right-hand side. BUG=v8:5267 R=mvstanton@chromium.org Committed: https://crrev.com/a0484bc6116ebc2b855de87d862945e2ae07169b Cr-Commit-Position: refs/heads/master@{#39718}

Patch Set 1 #

Patch Set 2 : Fix copy and paste. #

Patch Set 3 : Fix registers on arm/arm64. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+353 lines, -210 lines) Patch
M src/ast/ast.cc View 1 chunk +5 lines, -1 line 0 comments Download
M src/code-stub-assembler.h View 2 chunks +21 lines, -3 lines 0 comments Download
M src/code-stub-assembler.cc View 2 chunks +13 lines, -1 line 0 comments Download
M src/code-stubs.h View 2 chunks +12 lines, -0 lines 0 comments Download
M src/code-stubs.cc View 1 chunk +61 lines, -0 lines 0 comments Download
M src/compiler/ast-graph-builder.cc View 1 chunk +5 lines, -2 lines 0 comments Download
M src/compiler/bytecode-graph-builder.cc View 1 chunk +3 lines, -1 line 0 comments Download
M src/compiler/effect-control-linearizer.h View 1 chunk +5 lines, -2 lines 0 comments Download
M src/compiler/effect-control-linearizer.cc View 3 chunks +108 lines, -6 lines 0 comments Download
M src/compiler/js-operator.h View 3 chunks +4 lines, -1 line 0 comments Download
M src/compiler/js-operator.cc View 4 chunks +20 lines, -1 line 0 comments Download
M src/compiler/js-typed-lowering.cc View 2 chunks +18 lines, -123 lines 0 comments Download
M src/compiler/opcodes.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/compiler/simplified-lowering.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M src/compiler/simplified-operator.h View 1 chunk +2 lines, -0 lines 0 comments Download
M src/compiler/simplified-operator.cc View 1 chunk +1 line, -0 lines 0 comments Download
M src/compiler/typer.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M src/compiler/verifier.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M src/crankshaft/hydrogen.cc View 1 chunk +17 lines, -9 lines 0 comments Download
M src/full-codegen/arm/full-codegen-arm.cc View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M src/full-codegen/arm64/full-codegen-arm64.cc View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M src/full-codegen/full-codegen.h View 1 chunk +6 lines, -2 lines 0 comments Download
M src/full-codegen/ia32/full-codegen-ia32.cc View 1 chunk +3 lines, -1 line 0 comments Download
M src/full-codegen/mips/full-codegen-mips.cc View 1 1 chunk +3 lines, -1 line 0 comments Download
M src/full-codegen/mips64/full-codegen-mips64.cc View 1 1 chunk +3 lines, -1 line 0 comments Download
M src/full-codegen/x64/full-codegen-x64.cc View 1 chunk +3 lines, -1 line 0 comments Download
M src/interpreter/bytecode-array-builder.cc View 1 chunk +2 lines, -1 line 0 comments Download
M src/interpreter/bytecodes.h View 1 chunk +2 lines, -1 line 0 comments Download
M src/interpreter/interpreter.cc View 1 chunk +1 line, -1 line 0 comments Download
M test/cctest/interpreter/bytecode_expectations/IfConditions.golden View 2 chunks +2 lines, -2 lines 0 comments Download
A + test/mjsunit/compiler/deopt-instanceof-proxy.js View 1 chunk +13 lines, -10 lines 0 comments Download
M test/unittests/compiler/js-typed-lowering-unittest.cc View 1 chunk +0 lines, -37 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 21 (14 generated)
Benedikt Meurer
4 years, 2 months ago (2016-09-26 08:07:26 UTC) #1
Benedikt Meurer
Hey Michael, Here's a fix for instanceof deopt loops, which also allows TurboFan to optimize ...
4 years, 2 months ago (2016-09-26 08:08:29 UTC) #4
mvstanton
LGTM.
4 years, 2 months ago (2016-09-26 12:14:16 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2370693002/40001
4 years, 2 months ago (2016-09-26 12:16:16 UTC) #17
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 2 months ago (2016-09-26 12:32:28 UTC) #18
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/a0484bc6116ebc2b855de87d862945e2ae07169b Cr-Commit-Position: refs/heads/master@{#39718}
4 years, 2 months ago (2016-09-26 12:32:42 UTC) #20
Benedikt Meurer
4 years, 2 months ago (2016-09-26 17:39:56 UTC) #21
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2365223003/ by bmeurer@chromium.org.

The reason for reverting is: Tanks EarleyBoyer..

Powered by Google App Engine
This is Rietveld 408576698