Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2271443003:
[deoptimizer] Potentially deopt into debug bytecode. (Closed)
|
Created:
4 years, 4 months ago by Michael Starzinger Modified:
4 years, 4 months ago 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[deoptimizer] Potentially deopt into debug bytecode.
This makes sure the deoptimizer picks bytecode prepared for debugging
when materializing an interpreted frame if one is available. This is
normally done by the interpreter entry trampoline and hence needs to be
replicated by the deoptimizer.
R=jarin@chromium.org
Committed: https://crrev.com/09a7ac5fcaf795f07f8b8204d25184d38ba9cf6e
Cr-Commit-Position: refs/heads/master@{#38815}
Patch Set 1 #Patch Set 2 : Test expectation. #
Total comments: 2
Messages
Total messages: 21 (13 generated)
The CQ bit was checked by mstarzinger@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...) v8_linux64_gyp_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng_trigg...)
mstarzinger@chromium.org changed reviewers: + rmcilroy@chromium.org
Jaro: PTAL. Ross: FYI.
The CQ bit was checked by mstarzinger@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== [deoptimizer] Potential deopt into debug bytecode. This makes sure the deoptimizer picks bytecode prepared for debugging when materializing an interpreted frame if one is available. This is normally done by the interpreter entry trampoline and hence needs to be replicated by the deoptimizer. R=jarin@chromium.org ========== to ========== [deoptimizer] Potentially deopt into debug bytecode. This makes sure the deoptimizer picks bytecode prepared for debugging when materializing an interpreted frame if one is available. This is normally done by the interpreter entry trampoline and hence needs to be replicated by the deoptimizer. R=jarin@chromium.org ==========
lgtm. Thanks!
The CQ bit was checked by mstarzinger@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [deoptimizer] Potentially deopt into debug bytecode. This makes sure the deoptimizer picks bytecode prepared for debugging when materializing an interpreted frame if one is available. This is normally done by the interpreter entry trampoline and hence needs to be replicated by the deoptimizer. R=jarin@chromium.org ========== to ========== [deoptimizer] Potentially deopt into debug bytecode. This makes sure the deoptimizer picks bytecode prepared for debugging when materializing an interpreted frame if one is available. This is normally done by the interpreter entry trampoline and hence needs to be replicated by the deoptimizer. R=jarin@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [deoptimizer] Potentially deopt into debug bytecode. This makes sure the deoptimizer picks bytecode prepared for debugging when materializing an interpreted frame if one is available. This is normally done by the interpreter entry trampoline and hence needs to be replicated by the deoptimizer. R=jarin@chromium.org ========== to ========== [deoptimizer] Potentially deopt into debug bytecode. This makes sure the deoptimizer picks bytecode prepared for debugging when materializing an interpreted frame if one is available. This is normally done by the interpreter entry trampoline and hence needs to be replicated by the deoptimizer. R=jarin@chromium.org Committed: https://crrev.com/09a7ac5fcaf795f07f8b8204d25184d38ba9cf6e Cr-Commit-Position: refs/heads/master@{#38815} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/09a7ac5fcaf795f07f8b8204d25184d38ba9cf6e Cr-Commit-Position: refs/heads/master@{#38815}
Message was sent while issue was closed.
LGTM, thanks. https://codereview.chromium.org/2271443003/diff/20001/test/mjsunit/mjsunit.st... File test/mjsunit/mjsunit.status (right): https://codereview.chromium.org/2271443003/diff/20001/test/mjsunit/mjsunit.st... test/mjsunit/mjsunit.status:60: 'debug-step-turbofan': [PASS, FAIL], Could we move this to be a fail for the "not ignition_staging" variant and a pass for ignition_staging? If it ends up being complicated don't worry, this is fine.
Message was sent while issue was closed.
https://codereview.chromium.org/2271443003/diff/20001/test/mjsunit/mjsunit.st... File test/mjsunit/mjsunit.status (right): https://codereview.chromium.org/2271443003/diff/20001/test/mjsunit/mjsunit.st... test/mjsunit/mjsunit.status:60: 'debug-step-turbofan': [PASS, FAIL], On 2016/08/23 12:44:25, rmcilroy wrote: > Could we move this to be a fail for the "not ignition_staging" variant and a > pass for ignition_staging? If it ends up being complicated don't worry, this is > fine. I thought about doing this, but the problem is that the "default" variant is also failing (only passing for ignition_staging and ignition_turbofan). And I was wary about adding a section for the "default" variant. That seemed strange to me. WDYT?
Message was sent while issue was closed.
On 2016/08/23 12:47:08, Michael Starzinger wrote: > https://codereview.chromium.org/2271443003/diff/20001/test/mjsunit/mjsunit.st... > File test/mjsunit/mjsunit.status (right): > > https://codereview.chromium.org/2271443003/diff/20001/test/mjsunit/mjsunit.st... > test/mjsunit/mjsunit.status:60: 'debug-step-turbofan': [PASS, FAIL], > On 2016/08/23 12:44:25, rmcilroy wrote: > > Could we move this to be a fail for the "not ignition_staging" variant and a > > pass for ignition_staging? If it ends up being complicated don't worry, this > is > > fine. > > I thought about doing this, but the problem is that the "default" variant is > also failing (only passing for ignition_staging and ignition_turbofan). And I > was wary about adding a section for the "default" variant. That seemed strange > to me. WDYT? Yeah, I was thinking you could negate the variant to avoid specifying the default one, i.e.: if (variant != ignition_staging or variant != ignition_turbofan) { 'debug-step-turbofan': [FAIL], } But this probably doesn't work with the hard-coded variant parsing code. What you have is fine, still LGTM. (also didn't realize I'd sent the comments after it already landed, so no need to change anything) |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
