IonMonkey: Assertion failure: *from != *to, at CodeGenerator-x86-shared.cpp:213

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: adrake, Assigned: dvander)

Tracking

(Blocks 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Reporter

Description

8 years ago
Posted file Test case
With greedy allocator enabled on x86 debug:

[adrake@reappropriated obj32]$ ./js --ion critical.js 
Assertion failure: *from != *to, at /home/adrake/moz/ionmonkey/js/src/ion/shared/CodeGenerator-x86-shared.cpp:213
Aborted (core dumped)
The assertion was caused by a stack -> stack move during phi node move resolution, when both stacks happened to be equal. Patch just doesn't emit a move, and adds an assertion to cause failure closer to point of use.
Attachment #549295 - Flags: review?(adrake)
Reporter

Comment 2

8 years ago
Comment on attachment 549295 [details] [diff] [review]
Fix stack -> stack move where stacks are equal.

Review of attachment 549295 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #549295 - Flags: review?(adrake) → review+
http://hg.mozilla.org/projects/ionmonkey/rev/18eaec755ec5
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Reporter

Comment 4

8 years ago
Posted file Test case 2
This test case trips the new assert -- seems to be caused by phi resolution as well.
Reporter

Updated

8 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter

Updated

8 years ago
Blocks: anion
No longer blocks: IonMonkey
Duplicate of this bug: 671112
Posted patch fixSplinter Review
There are two bugs here.
 * Greedy allocator relies on block->id() < graph.numBlocks() and this regressed.
 * The register assignments to defs was left lying around after processing each
   block. We could make this work, clearing it lazily via peeking at |state|, but
   that strikes me as error-prone and scary. So now we just clear register
   assignments leaving a block, and restore them when merging state.

I also introduced some asserts to catch errors earlier.
Assignee: general → dvander
Attachment #549295 - Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Attachment #550835 - Flags: review?(adrake)
Reporter

Comment 7

8 years ago
Comment on attachment 550835 [details] [diff] [review]
fix

Review of attachment 550835 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ion/GreedyAllocator.h
@@ +301,5 @@
>      RegisterSet allocatableRegs() const {
>          return RegisterSet::Intersect(state.free, RegisterSet::Not(disallowed));
>      }
>      BlockInfo *blockInfo(LBlock *block) {
> +        JS_ASSERT(block->mir()->id() < graph.numBlocks());

This should be graph.numBlockIds() for sparse block IDs.
Attachment #550835 - Flags: review?(adrake) → review+
http://hg.mozilla.org/projects/ionmonkey/rev/2ee4cdc9856b
Status: ASSIGNED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.