Closed
Bug 1133149
Opened 9 years ago
Closed 7 years ago
Further testcase for "Assertion failure: phi->hasUses() (Missed an unused phi)"
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox38 | --- | affected |
People
(Reporter: azakai, Assigned: sunfish)
References
Details
(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update,testComment=2,origRev=81f979b17fbd,ignore])
Attachments
(2 files)
491.74 KB,
text/x-csrc
|
Details | |
1.13 KB,
patch
|
Details | Diff | Splinter Review |
Even after bug 1117882 landed, I see this assert. Attached is a new testcase showing it.
Assignee | ||
Comment 1•9 years ago
|
||
Range analysis sometimes sets isGuard on arbitrary instructions. When it does this to a phi, we can't delete it, so disable the assert and the rerun check in that case.
Assignee: nobody → sunfish
Attachment #8564450 -
Flags: review?(nicolas.b.pierron)
(function() { 'use asm' function z() { var c = 0 var d = 0 var e = 0 var g = 0 var a = 0 var b = 0 var f = 0 while (1) { a = _() | 0 b = (a << 16) > -4 if (b) {} else { c = a break } } d = (c << 16) == 0 while (1) { if (!(d)) { break } e = (f - 1) | 0 g = (f | 0) > 0 if (g) { f = e } else break } } })() asserts js debug shell on m-c rev 81f979b17fbd with --fuzzing-safe --no-threads --ion-eager at Assertion failure: phi->hasUses() (Missed an unused phi), at jit/ValueNumbering.cpp === Treeherder Build Bisection Results by autoBisect === The "good" changeset has the timestamp "20150202082836" and the hash "c3714f520752". The "bad" changeset has the timestamp "20150202091038" and the hash "db24c7441b79". Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c3714f520752&tochange=db24c7441b79
Blocks: 1122402
Severity: normal → critical
status-firefox38:
--- → affected
OS: Linux → All
Whiteboard: [jsbugmon:update,testComment=2,origRev=81f979b17fbd]
Version: unspecified → Trunk
Comment 3•9 years ago
|
||
(In reply to Dan Gohman [:sunfish] from comment #1) > Range analysis sometimes sets isGuard on arbitrary instructions. When it > does this to a phi, we can't delete it, so disable the assert and the rerun > check in that case. Sounds that this is bogus and that the operands of the Phi nodes should have this guard flag instead of the Phi. It does not make sense to flag an MPhi as a guard as it-self, it does not filter any input.
Comment 4•9 years ago
|
||
Comment on attachment 8564450 [details] [diff] [review] gvn-guard-phi.patch Review of attachment 8564450 [details] [diff] [review]: ----------------------------------------------------------------- Could we set the Guard flag on the operands instead of setting it on the MPhi?
Attachment #8564450 -
Flags: review?(nicolas.b.pierron)
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Nicolas B. Pierron [:nbp] from comment #4) > Could we set the Guard flag on the operands instead of setting it on the > MPhi? What if the operands are themselves phis?
Comment 6•9 years ago
|
||
(In reply to Dan Gohman [:sunfish] from comment #5) > (In reply to Nicolas B. Pierron [:nbp] from comment #4) > > Could we set the Guard flag on the operands instead of setting it on the > > MPhi? > > What if the operands are themselves phis? Then we should push these flags to their operands, and avoid loops. Another way would be to push the Guard flag to the operands when we are removing unused Phi.
Updated•9 years ago
|
Whiteboard: [jsbugmon:update,testComment=2,origRev=81f979b17fbd] → [jsbugmon:update,testComment=2,origRev=81f979b17fbd,ignore]
Comment 7•9 years ago
|
||
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 993eb76a8bd6).
Not sure what's next here.
Flags: needinfo?(sunfish)
Assignee | ||
Comment 9•9 years ago
|
||
This appears to be fixed in 31d7b208abd1 (bug 1130679). Range analysis is no longer setting the isGuard flag in the same way.
Flags: needinfo?(sunfish)
Comment 10•7 years ago
|
||
(In reply to Dan Gohman [:sunfish] from comment #9) > This appears to be fixed in 31d7b208abd1 (bug 1130679). Range analysis is no > longer setting the isGuard flag in the same way. Closing then :)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•