|
|
Created:
3 years, 10 months ago by Mircea Trofin Modified:
3 years, 10 months ago CC:
v8-reviews_googlegroups.com Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[turbofan] more regalloc fixes
BUG=v8:5911
Review-Url: https://codereview.chromium.org/2667963004
Cr-Commit-Position: refs/heads/master@{#42900}
Committed: https://chromium.googlesource.com/v8/v8/+/b0e58a9cee42fe4881b7d8b3cd1a06178c4c34ea
Patch Set 1 #Patch Set 2 : [turbofan] more regalloc fixes #
Total comments: 2
Messages
Total messages: 20 (13 generated)
The CQ bit was checked by mtrofin@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 checked by mtrofin@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.
titzer@chromium.org changed reviewers: + titzer@chromium.org
lgtm
eholk@chromium.org changed reviewers: + eholk@chromium.org
https://codereview.chromium.org/2667963004/diff/20001/test/mjsunit/regress/re... File test/mjsunit/regress/regress-5911.js (right): https://codereview.chromium.org/2667963004/diff/20001/test/mjsunit/regress/re... test/mjsunit/regress/regress-5911.js:27: kExprUnreachable, It looks like this test unconditionally hits this unreachable instruction. If so, how does this test detect the bug?
Description was changed from ========== [turbofan] more regalloc fixes BUG= ========== to ========== [turbofan] more regalloc fixes BUG=v8:5911 ==========
ahaas@chromium.org changed reviewers: + ahaas@chromium.org
https://codereview.chromium.org/2667963004/diff/20001/test/mjsunit/regress/re... File test/mjsunit/regress/regress-5911.js (right): https://codereview.chromium.org/2667963004/diff/20001/test/mjsunit/regress/re... test/mjsunit/regress/regress-5911.js:27: kExprUnreachable, On 2017/02/01 at 19:04:25, Eric Holk wrote: > It looks like this test unconditionally hits this unreachable instruction. If so, how does this test detect the bug? This test case crashed during compilation, not during execution. The unreachable instruction can be helpful for compilation because with the unreachable it is more likely that a WebAssembly program validates.
The CQ bit was checked by mtrofin@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/02/02 20:21:42, ahaas wrote: > https://codereview.chromium.org/2667963004/diff/20001/test/mjsunit/regress/re... > File test/mjsunit/regress/regress-5911.js (right): > > https://codereview.chromium.org/2667963004/diff/20001/test/mjsunit/regress/re... > test/mjsunit/regress/regress-5911.js:27: kExprUnreachable, > On 2017/02/01 at 19:04:25, Eric Holk wrote: > > It looks like this test unconditionally hits this unreachable instruction. If > so, how does this test detect the bug? > > This test case crashed during compilation, not during execution. The unreachable > instruction can be helpful for compilation because with the unreachable it is > more likely that a WebAssembly program validates. Indeed. Committing after separate offline fuzzer validation (thanks, Andreas!)
On 2017/02/02 20:55:50, Mircea Trofin wrote: > On 2017/02/02 20:21:42, ahaas wrote: > > > https://codereview.chromium.org/2667963004/diff/20001/test/mjsunit/regress/re... > > File test/mjsunit/regress/regress-5911.js (right): > > > > > https://codereview.chromium.org/2667963004/diff/20001/test/mjsunit/regress/re... > > test/mjsunit/regress/regress-5911.js:27: kExprUnreachable, > > On 2017/02/01 at 19:04:25, Eric Holk wrote: > > > It looks like this test unconditionally hits this unreachable instruction. > If > > so, how does this test detect the bug? > > > > This test case crashed during compilation, not during execution. The > unreachable > > instruction can be helpful for compilation because with the unreachable it is > > more likely that a WebAssembly program validates. > > Indeed. > > Committing after separate offline fuzzer validation (thanks, Andreas!) ...and also thanks to Erik, who ran his fuzzer as well.
CQ is committing da patch. Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486068909951260, "parent_rev": "dbda66ec4f7f26b7870ebc0b7b5d348573d7b9aa", "commit_rev": "b0e58a9cee42fe4881b7d8b3cd1a06178c4c34ea"}
Message was sent while issue was closed.
Description was changed from ========== [turbofan] more regalloc fixes BUG=v8:5911 ========== to ========== [turbofan] more regalloc fixes BUG=v8:5911 Review-Url: https://codereview.chromium.org/2667963004 Cr-Commit-Position: refs/heads/master@{#42900} Committed: https://chromium.googlesource.com/v8/v8/+/b0e58a9cee42fe4881b7d8b3cd1a06178c4... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/v8/v8/+/b0e58a9cee42fe4881b7d8b3cd1a06178c4... |