Chromium Code Reviews
|
|
Chromium Code Reviews|
Created:
3 years, 11 months ago by Tobias Tebbi Modified:
3 years, 10 months ago Reviewers:
*Benedikt Meurer, Michael Starzinger CC:
v8-reviews_googlegroups.com Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[tubofan] teach escape analysis about ObjectIsNonCallable
R=mstarzinger@chromium.org, bmeurer@chromium.org
BUG=chromium:669242
Review-Url: https://codereview.chromium.org/2645273003
Cr-Commit-Position: refs/heads/master@{#42630}
Committed: https://chromium.googlesource.com/v8/v8/+/e225251f256472562e1fc79feec2d7203b15e443
Patch Set 1 #Patch Set 2 : added missing opcode #
Total comments: 2
Patch Set 3 : restore strict check #
Created: 3 years, 11 months ago
Messages
Total messages: 16 (8 generated)
LGTM from my end. Benedikt, you had an opinion about keeping this list explicit in the past. Are you fine with changing it to a DCHECK, in order to "hide" the issues from Chrome crashes in the wild?
Description was changed from ========== [tubofan] escape analysis crashes only in debug builds for unaccounted opcodes R=mstarzinger@chromium.org BUG=chromium:669242 ========== to ========== [tubofan] escape analysis crashes only in debug builds for unaccounted opcodes R=mstarzinger@chromium.org BUG=chromium:669242 ==========
mstarzinger@chromium.org changed reviewers: + bmeurer@chromium.org
mstarzinger@chromium.org changed required reviewers: + bmeurer@chromium.org
LGTM % nits https://codereview.chromium.org/2645273003/diff/20001/src/compiler/escape-ana... File src/compiler/escape-analysis.cc (right): https://codereview.chromium.org/2645273003/diff/20001/src/compiler/escape-ana... src/compiler/escape-analysis.cc:840: PrintF("Encountered unaccounted use by #%d (%s)\n", use->id(), Nit: Do V8_Fatal here please. https://codereview.chromium.org/2645273003/diff/20001/src/compiler/escape-ana... src/compiler/escape-analysis.cc:842: DCHECK(false); Nit: Please keep UNREACHABLE()
Description was changed from ========== [tubofan] escape analysis crashes only in debug builds for unaccounted opcodes R=mstarzinger@chromium.org BUG=chromium:669242 ========== to ========== [tubofan] teach escape analysis about ObjectIsNonCallable R=mstarzinger@chromium.org, bmeurer@chromium.org BUG=chromium:669242 ==========
The CQ bit was checked by tebbi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bmeurer@chromium.org, mstarzinger@chromium.org Link to the patchset: https://codereview.chromium.org/2645273003/#ps40001 (title: "restore strict check")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485261906892840,
"parent_rev": "a920c0d92bd5d1c59a23ba4c103c96d7724d0460", "commit_rev":
"e225251f256472562e1fc79feec2d7203b15e443"}
Message was sent while issue was closed.
Description was changed from ========== [tubofan] teach escape analysis about ObjectIsNonCallable R=mstarzinger@chromium.org, bmeurer@chromium.org BUG=chromium:669242 ========== to ========== [tubofan] teach escape analysis about ObjectIsNonCallable R=mstarzinger@chromium.org, bmeurer@chromium.org BUG=chromium:669242 Review-Url: https://codereview.chromium.org/2645273003 Cr-Commit-Position: refs/heads/master@{#42630} Committed: https://chromium.googlesource.com/v8/v8/+/e225251f256472562e1fc79feec2d7203b1... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/v8/v8/+/e225251f256472562e1fc79feec2d7203b1...
Message was sent while issue was closed.
Note, there is an escape analysis error fired flakily since today, e.g.: https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20shared/build...
Message was sent while issue was closed.
On 2017/01/26 22:29:44, Michael Achenbach wrote: > Note, there is an escape analysis error fired flakily since today, e.g.: > https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20shared/build... Bisected. Culprit seems to be this CL: https://codereview.chromium.org/2638393002/
Message was sent while issue was closed.
On 2017/01/27 09:09:55, Michael Achenbach wrote: > On 2017/01/26 22:29:44, Michael Achenbach wrote: > > Note, there is an escape analysis error fired flakily since today, e.g.: > > > https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20shared/build... > > Bisected. Culprit seems to be this CL: > https://codereview.chromium.org/2638393002/ Also note that ClusterFuzz has several repro cases for this assertion. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
