Closed Bug 677066 Opened 8 years ago Closed 8 years ago
Monkey: Emitting instructions at uses during lowering is incorrect in some cases
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.
8 years ago
Assignee: general → dvander
Status: NEW → ASSIGNED
8 years ago
Depends on: 677380
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.
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) <|@
Thanks, there was a stupid typo, op->type() instead of op->id()
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+
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
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)
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.