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

Issue 1425883004: [turbofan] Fix missing bailout point before calls. (Closed)

Created:
5 years, 1 month ago by Benedikt Meurer
Modified:
5 years, 1 month ago
Reviewers:
Yang
CC:
v8-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[turbofan] Fix missing bailout point before calls. In order to properly (lazy) bailout when converting the receiver for sloppy mode functions (using the newly added JSConvertReceiver operator), we need to have a bailout location right before every call (also right before every %_Call and %_CallFunction), otherwise if the JSConvertReceiver just reuses the lazy bailout frame state from the JSCallFunction node, it will skip the whole function in case of lazy bailout. Note it should be impossible to trigger this currently because we do not yet support AllocationSite code dependencies in TurboFan, which can trigger this kind of lazy bailout; therefore it's not possible to write a regression test (yet). R=yangguo@chromium.org BUG=v8:4493 LOG=n Committed: https://crrev.com/6040d5c0db96b38bd3008bc4cd644dd1d87aa4db Cr-Commit-Position: refs/heads/master@{#31668}

Patch Set 1 #

Patch Set 2 : Update unittests. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+135 lines, -76 lines) Patch
M src/ast.h View 1 chunk +2 lines, -1 line 0 comments Download
M src/compiler/ast-graph-builder.cc View 4 chunks +11 lines, -5 lines 0 comments Download
M src/compiler/js-generic-lowering.cc View 1 chunk +1 line, -0 lines 0 comments Download
M src/compiler/js-inlining.cc View 4 chunks +26 lines, -17 lines 0 comments Download
M src/compiler/js-typed-lowering.cc View 1 chunk +1 line, -0 lines 0 comments Download
M src/compiler/linkage.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M src/compiler/node-properties.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/compiler/node-properties.cc View 1 chunk +7 lines, -0 lines 0 comments Download
M src/compiler/operator-properties.cc View 1 chunk +5 lines, -1 line 0 comments Download
M src/full-codegen/arm/full-codegen-arm.cc View 3 chunks +3 lines, -0 lines 0 comments Download
M src/full-codegen/arm64/full-codegen-arm64.cc View 3 chunks +3 lines, -0 lines 0 comments Download
M src/full-codegen/ia32/full-codegen-ia32.cc View 3 chunks +3 lines, -0 lines 0 comments Download
M src/full-codegen/mips/full-codegen-mips.cc View 3 chunks +3 lines, -0 lines 0 comments Download
M src/full-codegen/mips64/full-codegen-mips64.cc View 3 chunks +3 lines, -0 lines 0 comments Download
M src/full-codegen/ppc/full-codegen-ppc.cc View 3 chunks +3 lines, -0 lines 0 comments Download
M src/full-codegen/x64/full-codegen-x64.cc View 3 chunks +3 lines, -0 lines 0 comments Download
M src/full-codegen/x87/full-codegen-x87.cc View 3 chunks +3 lines, -0 lines 0 comments Download
M test/unittests/compiler/js-builtin-reducer-unittest.cc View 1 7 chunks +15 lines, -10 lines 0 comments Download
M test/unittests/compiler/js-context-relaxation-unittest.cc View 1 10 chunks +40 lines, -40 lines 0 comments Download

Messages

Total messages: 8 (2 generated)
Benedikt Meurer
5 years, 1 month ago (2015-10-30 06:17:48 UTC) #1
Benedikt Meurer
Hey Yang, Here's the fix for the funny "skip function invocation" bailout I told you ...
5 years, 1 month ago (2015-10-30 06:18:52 UTC) #2
Yang
lgtm. Maybe a test case?
5 years, 1 month ago (2015-10-30 06:31:50 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1425883004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1425883004/20001
5 years, 1 month ago (2015-10-30 06:41:29 UTC) #6
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 1 month ago (2015-10-30 06:59:02 UTC) #7
commit-bot: I haz the power
5 years, 1 month ago (2015-10-30 06:59:17 UTC) #8
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/6040d5c0db96b38bd3008bc4cd644dd1d87aa4db
Cr-Commit-Position: refs/heads/master@{#31668}

Powered by Google App Engine
This is Rietveld 408576698