Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2488673004:
[ic] Support data handlers that represent transitioning stores. (Closed)
|
Created:
4 years, 1 month ago by Igor Sheludko Modified:
4 years, 1 month ago CC:
v8-reviews_googlegroups.com Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[ic] Support data handlers that represent transitioning stores.
BUG=v8:5561
Committed: https://crrev.com/bcb3af59be2c52ca13a0adae500f042a64093b6d
Cr-Commit-Position: refs/heads/master@{#40946}
Patch Set 1 #
Total comments: 8
Patch Set 2 : Addressing comments #Patch Set 3 : Rebasing #Patch Set 4 : Rebasing again #
Messages
Total messages: 49 (31 generated)
Patchset #1 (id:1) has been deleted
The CQ bit was checked by ishell@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...
Description was changed from ========== [ic] Enable data handlers for transitioning stores. BUG=v8:5561 ========== to ========== [ic] Support data handlers that represent transitioning stores. BUG=v8:5561 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...) v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...) 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_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/15896) v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...) v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...) v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/15934) v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...) v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...) v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/15857) v8_presubmit on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/28165)
ishell@chromium.org changed reviewers: + jkummerow@chromium.org
PTAL
Looks good. https://codereview.chromium.org/2488673004/diff/20001/src/ic/ic.cc File src/ic/ic.cc (right): https://codereview.chromium.org/2488673004/diff/20001/src/ic/ic.cc#newcode1836 src/ic/ic.cc:1836: // Stub is never generated for objects that require access checks. No stubs are generated at all ;-) Suggestion: "Declarative handlers don't support access checks." https://codereview.chromium.org/2488673004/diff/20001/src/ic/ic.cc#newcode1857 src/ic/ic.cc:1857: // Stub is never generated for objects that require access checks. You've already checked this above. https://codereview.chromium.org/2488673004/diff/20001/src/ic/ic.cc#newcode1877 src/ic/ic.cc:1877: LoadHandler::kFirstPrototypeIndex + checks_count, TENURED)); s/LoadHandler/StoreHandler/ here and in the next three lines https://codereview.chromium.org/2488673004/diff/20001/src/ic/ic.cc#newcode1880 src/ic/ic.cc:1880: handler_array->set(LoadHandler::kHolderCellIndex, *transition_cell); s/kHolderCellIndex/kTransitionCellIndex/
https://codereview.chromium.org/2488673004/diff/20001/src/ic/ic.cc File src/ic/ic.cc (right): https://codereview.chromium.org/2488673004/diff/20001/src/ic/ic.cc#newcode1836 src/ic/ic.cc:1836: // Stub is never generated for objects that require access checks. On 2016/11/10 14:13:07, Jakob Kummerow wrote: > No stubs are generated at all ;-) Suggestion: > "Declarative handlers don't support access checks." Done. https://codereview.chromium.org/2488673004/diff/20001/src/ic/ic.cc#newcode1857 src/ic/ic.cc:1857: // Stub is never generated for objects that require access checks. On 2016/11/10 14:13:07, Jakob Kummerow wrote: > You've already checked this above. Done. https://codereview.chromium.org/2488673004/diff/20001/src/ic/ic.cc#newcode1877 src/ic/ic.cc:1877: LoadHandler::kFirstPrototypeIndex + checks_count, TENURED)); On 2016/11/10 14:13:07, Jakob Kummerow wrote: > s/LoadHandler/StoreHandler/ here and in the next three lines Done. https://codereview.chromium.org/2488673004/diff/20001/src/ic/ic.cc#newcode1880 src/ic/ic.cc:1880: handler_array->set(LoadHandler::kHolderCellIndex, *transition_cell); On 2016/11/10 14:13:07, Jakob Kummerow wrote: > s/kHolderCellIndex/kTransitionCellIndex/ Done.
The CQ bit was checked by ishell@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.
lgtm
The CQ bit was checked by ishell@chromium.org
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
Try jobs failed on following builders: v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...) v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...) v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...) v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
The CQ bit was checked by ishell@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jkummerow@chromium.org Link to the patchset: https://codereview.chromium.org/2488673004/#ps80001 (title: "Rebasing again")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #4 (id:80001) has been deleted
Try jobs failed on following builders: v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...) v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...) v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/12417) v8_presubmit on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/28420)
The CQ bit was checked by ishell@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jkummerow@chromium.org Link to the patchset: https://codereview.chromium.org/2488673004/#ps100001 (title: "Rebasing again")
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
Try jobs failed on following builders: v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...) v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...) v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...) v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...) v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/17659)
Patchset #5 (id:120001) has been deleted
The CQ bit was checked by ishell@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_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...) v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...) v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...) v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
The CQ bit was checked by ishell@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jkummerow@chromium.org Link to the patchset: https://codereview.chromium.org/2488673004/#ps100001 (title: "Rebasing again")
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
Try jobs failed on following builders: v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...) v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
The CQ bit was checked by ishell@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 ========== [ic] Support data handlers that represent transitioning stores. BUG=v8:5561 ========== to ========== [ic] Support data handlers that represent transitioning stores. BUG=v8:5561 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:100001)
Message was sent while issue was closed.
jochen@chromium.org changed reviewers: + jochen@chromium.org
Message was sent while issue was closed.
there's a large bump in Group-Callback on youtube-polymer/ignition. Any idea what's going on here?
Message was sent while issue was closed.
On 2016/11/16 22:05:33, jochen (travelling til Nov 18) wrote: > there's a large bump in Group-Callback on youtube-polymer/ignition. Any idea > what's going on here? Please also fix: https://build.chromium.org/p/client.v8.ports/builders/V8%20Linux%20-%20arm64%... Unreachable code.
Message was sent while issue was closed.
On 2016/11/17 07:41:01, machenbach (slow) wrote: > On 2016/11/16 22:05:33, jochen (travelling til Nov 18) wrote: > > there's a large bump in Group-Callback on youtube-polymer/ignition. Any idea > > what's going on here? > > Please also fix: > https://build.chromium.org/p/client.v8.ports/builders/V8%20Linux%20-%20arm64%... > Unreachable code. This test is outdated. Ulan will remove it.
Message was sent while issue was closed.
Description was changed from ========== [ic] Support data handlers that represent transitioning stores. BUG=v8:5561 ========== to ========== [ic] Support data handlers that represent transitioning stores. BUG=v8:5561 Committed: https://crrev.com/bcb3af59be2c52ca13a0adae500f042a64093b6d Cr-Commit-Position: refs/heads/master@{#40946} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/bcb3af59be2c52ca13a0adae500f042a64093b6d Cr-Commit-Position: refs/heads/master@{#40946} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
