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

Issue 1340313003: [turbofan] Make arguments object materialization inlinable. (Closed)

Created:
5 years, 3 months ago by Michael Starzinger
Modified:
5 years, 3 months ago
Reviewers:
mvstanton
CC:
v8-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@local_turbofan-arguments-2
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[turbofan] Make arguments object materialization inlinable. This makes sure that the arguments object materialization in the method prologue is composable with respect to inlining. The generic runtime functions materializing those objects now respect the deoptimization information when reconstructing the original arguments. R=mvstanton@chromium.org Committed: https://crrev.com/2c54dbda35d200e0d4e75629abe219060f15992e Cr-Commit-Position: refs/heads/master@{#30766}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Added tests. #

Patch Set 3 : Clarified comment. #

Patch Set 4 : Rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+158 lines, -73 lines) Patch
M src/compiler/ast-graph-builder.cc View 3 chunks +8 lines, -6 lines 0 comments Download
M src/compiler/js-generic-lowering.cc View 2 chunks +16 lines, -1 line 0 comments Download
M src/compiler/js-inlining.cc View 1 2 3 2 chunks +3 lines, -12 lines 0 comments Download
M src/compiler/js-operator.h View 1 chunk +3 lines, -0 lines 0 comments Download
M src/compiler/js-operator.cc View 1 chunk +7 lines, -0 lines 0 comments Download
M src/compiler/linkage.cc View 1 chunk +0 lines, -1 line 0 comments Download
M src/compiler/operator-properties.cc View 1 chunk +1 line, -0 lines 0 comments Download
M src/runtime/runtime.h View 1 2 3 1 chunk +25 lines, -24 lines 0 comments Download
M src/runtime/runtime-scopes.cc View 1 2 3 9 chunks +58 lines, -27 lines 0 comments Download
M test/cctest/compiler/test-run-inlining.cc View 1 4 chunks +37 lines, -2 lines 0 comments Download

Messages

Total messages: 9 (2 generated)
Michael Starzinger
Note that test coverage for this is still missing, I will write test-run-inlining.cc tests to ...
5 years, 3 months ago (2015-09-15 14:21:50 UTC) #1
Michael Starzinger
Added tests and adapted JSInliner as well. Ready to roll.
5 years, 3 months ago (2015-09-15 15:02:58 UTC) #2
mvstanton
Nice! LGTM, one comment confused me a moment. https://codereview.chromium.org/1340313003/diff/1/src/runtime/runtime-scopes.cc File src/runtime/runtime-scopes.cc (right): https://codereview.chromium.org/1340313003/diff/1/src/runtime/runtime-scopes.cc#newcode545 src/runtime/runtime-scopes.cc:545: // ...
5 years, 3 months ago (2015-09-16 06:54:22 UTC) #3
Michael Starzinger
Thanks! Comments addressed. https://codereview.chromium.org/1340313003/diff/1/src/runtime/runtime-scopes.cc File src/runtime/runtime-scopes.cc (right): https://codereview.chromium.org/1340313003/diff/1/src/runtime/runtime-scopes.cc#newcode545 src/runtime/runtime-scopes.cc:545: // inlined, we use the slow ...
5 years, 3 months ago (2015-09-16 07:54:11 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1340313003/50001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1340313003/50001
5 years, 3 months ago (2015-09-16 13:02:42 UTC) #7
commit-bot: I haz the power
Committed patchset #4 (id:50001)
5 years, 3 months ago (2015-09-16 13:04:32 UTC) #8
commit-bot: I haz the power
5 years, 3 months ago (2015-09-16 13:04:43 UTC) #9
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/2c54dbda35d200e0d4e75629abe219060f15992e
Cr-Commit-Position: refs/heads/master@{#30766}

Powered by Google App Engine
This is Rietveld 408576698