Closed
Bug 677066
Opened 13 years ago
Closed 13 years ago
IonMonkey: Emitting instructions at uses during lowering is incorrect in some cases
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: adrake, Assigned: dvander)
References
(Blocks 2 open bugs)
Details
Attachments
(3 files, 1 obsolete file)
A bunch of issues have come up as a result of emitAtUses. 1) Given a control flow graph: /->3-\ 1- ->4 \->2-/ A constant / box defined in block 1, used in block 2, and used as a phi input in block 4 (from the block 3 edge) can get emitted at use in block 2 and not in block 1 or 3. As a result, the phi input is bogus on that edge. In this case (see attached), the greedy allocator produces a program that returns a bogus value, and the linear scan allocator trips the assertion added in bug 677041 due to an escaping live variable. 2) Emit at uses changes the ID of the original instruction at every emission. Because phis are lowered after all other instructions, phis get the most recent ID, which does not necessarily dominate the appropriate predecessor of the phi. I have a test case for this one lying around somewhere, when I find it I'll attach it.
Assignee | ||
Updated•13 years ago
|
Assignee: general → dvander
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•13 years ago
|
||
Andrew and I both agreed the problem here is how phis are lowered. In this patch: * All LBlocks and LPhis are created up-front, before any lowering. * Phis are defined when we reach the top of a block, but not lowered. * Inputs to phis are lowered individually at their incoming edges.
Attachment #551675 -
Flags: review?(adrake)
Reporter | ||
Comment 4•13 years ago
|
||
Fix regresses this test case. B3 looks particularly suspicious: 10 phi ([t:10]) (v7:*), (v7:*) <|@ 11 phi ([d:11]) (v7:*), (v4:*) <|@ 12 return () (v10:ecx), (v11:edx) <|@
Assignee | ||
Comment 5•13 years ago
|
||
Thanks, there was a stupid typo, op->type() instead of op->id()
Attachment #551675 -
Attachment is obsolete: true
Attachment #551675 -
Flags: review?(adrake)
Attachment #551750 -
Flags: review?(adrake)
Reporter | ||
Comment 6•13 years ago
|
||
Comment on attachment 551750 [details] [diff] [review] fix Review of attachment 551750 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, and passes all the test cases I've got!
Attachment #551750 -
Flags: review?(adrake) → review+
Assignee | ||
Comment 7•13 years ago
|
||
http://hg.mozilla.org/projects/ionmonkey/rev/a82301dbc678
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 8•13 years ago
|
||
Tried "testcase 2" (x64) on tip and on the pushed revision. On both I get the error: ra82301dbc678: Assertion failure: live->empty(), at /home/h4writer/Build/ionmonkey/js/src/ion/LinearScan.cpp:475 Aborted r29bd198d1ad1: Assertion failure: live->empty(), at /home/h4writer/Build/ionmonkey/js/src/ion/LinearScan.cpp:514 Aborted (This error was marked a duplicate of this bug, so that's why I'm reporting this in this bug)
Comment 9•13 years ago
|
||
Filed new bug for that bug (#678072). Note: it is another testcase and not "testcase 2" I mentioned in the previous comment.
You need to log in
before you can comment on or make changes to this bug.
Description
•