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 1604923002: [Interpreter] Always store current context in the frames context slot. (Closed)

Created:
4 years, 11 months ago by rmcilroy
Modified:
4 years, 11 months ago
Reviewers:
oth, Michael Starzinger
CC:
v8-reviews_googlegroups.com, oth
Base URL:
https://chromium.googlesource.com/v8/v8.git@int_fix_block_context_scope
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[Interpreter] Always store current context in the frames context slot. Change the interpreter to always store the current context in the frame's context slot instead of the function context. This makes it possible to restore the correct context during deopt. BUG=v8:4678, v8:4280 LOG=N Committed: https://crrev.com/677e54e24425b8d1b72d019e81c2cef8ff23060e Cr-Commit-Position: refs/heads/master@{#33477}

Patch Set 1 #

Patch Set 2 : Rebase and remove TODO from builtins #

Patch Set 3 : Rebase for popped context CL #

Patch Set 4 : Rebase #

Patch Set 5 : Fix try/catch tests #

Patch Set 6 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+206 lines, -186 lines) Patch
M src/arm/builtins-arm.cc View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M src/arm64/builtins-arm64.cc View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M src/compiler/bytecode-graph-builder.cc View 1 2 3 4 5 2 chunks +6 lines, -5 lines 0 comments Download
M src/compiler/interpreter-assembler.cc View 1 2 3 1 chunk +4 lines, -1 line 0 comments Download
M src/ia32/builtins-ia32.cc View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M src/interpreter/bytecode-array-builder.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M src/interpreter/bytecode-generator.h View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
M src/interpreter/bytecode-generator.cc View 1 2 3 4 5 4 chunks +14 lines, -21 lines 0 comments Download
M src/interpreter/bytecodes.h View 1 2 3 4 5 1 chunk +3 lines, -3 lines 0 comments Download
M src/interpreter/bytecodes.cc View 1 3 chunks +6 lines, -6 lines 0 comments Download
M src/interpreter/interpreter.cc View 1 2 3 4 5 1 chunk +6 lines, -4 lines 0 comments Download
M src/mips/builtins-mips.cc View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M src/mips64/builtins-mips64.cc View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M src/ppc/builtins-ppc.cc View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M src/x64/builtins-x64.cc View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M src/x87/builtins-x87.cc View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M test/cctest/interpreter/test-bytecode-generator.cc View 1 2 3 4 5 47 chunks +134 lines, -128 lines 0 comments Download
A test/mjsunit/deopt-with-outer-context.js View 1 chunk +22 lines, -0 lines 0 comments Download
M test/unittests/compiler/interpreter-assembler-unittest.cc View 1 1 chunk +10 lines, -0 lines 0 comments Download

Messages

Total messages: 36 (17 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1604923002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1604923002/1
4 years, 11 months ago (2016-01-19 14:20:55 UTC) #2
rmcilroy
Orion / Michi, please take a look thanks.
4 years, 11 months ago (2016-01-19 14:21:14 UTC) #4
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: v8_linux64_avx2_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel/builds/8904)
4 years, 11 months ago (2016-01-19 14:39:08 UTC) #6
oth
nice. lgtm.
4 years, 11 months ago (2016-01-19 16:18:00 UTC) #7
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1604923002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1604923002/20001
4 years, 11 months ago (2016-01-19 16:31:08 UTC) #9
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: v8_win64_rel_ng on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/1764) v8_win64_rel_ng_triggered on ...
4 years, 11 months ago (2016-01-19 16:51:38 UTC) #11
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1604923002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1604923002/40001
4 years, 11 months ago (2016-01-21 12:38:54 UTC) #13
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: v8_linux64_rel_ng on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/320) v8_win_nosnap_shared_compile_rel on ...
4 years, 11 months ago (2016-01-21 12:41:21 UTC) #15
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1604923002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1604923002/60001
4 years, 11 months ago (2016-01-22 11:49:42 UTC) #17
rmcilroy
Could you take a look Michi?
4 years, 11 months ago (2016-01-22 11:49:53 UTC) #18
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: v8_linux64_rel_ng on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/381)
4 years, 11 months ago (2016-01-22 11:53:50 UTC) #20
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1604923002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1604923002/80001
4 years, 11 months ago (2016-01-22 12:39:19 UTC) #22
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 11 months ago (2016-01-22 13:50:22 UTC) #24
Michael Starzinger
LGTM.
4 years, 11 months ago (2016-01-22 15:42:47 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1604923002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1604923002/80001
4 years, 11 months ago (2016-01-22 17:06:26 UTC) #28
commit-bot: I haz the power
Try jobs failed on following builders: v8_presubmit on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/10069)
4 years, 11 months ago (2016-01-22 17:08:36 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1604923002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1604923002/100001
4 years, 11 months ago (2016-01-22 21:42:53 UTC) #33
commit-bot: I haz the power
Committed patchset #6 (id:100001)
4 years, 11 months ago (2016-01-23 00:07:31 UTC) #34
commit-bot: I haz the power
4 years, 11 months ago (2016-01-23 00:07:56 UTC) #36
Message was sent while issue was closed.
Patchset 6 (id:??) landed as
https://crrev.com/677e54e24425b8d1b72d019e81c2cef8ff23060e
Cr-Commit-Position: refs/heads/master@{#33477}

Powered by Google App Engine
This is Rietveld 408576698