Closed
Bug 724975
Opened 12 years ago
Closed 12 years ago
IonMonkey: Assertion failure: loop_->loopHeader() == block->loopHeaderOfBackedge(), at ion/AliasAnalysis.cpp:165
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: decoder, Assigned: jandem)
References
Details
(Keywords: assertion, testcase)
Attachments
(3 files, 1 obsolete file)
623 bytes,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
12.24 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
2.97 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
The following testcase asserts on ionmonkey revision c34398f961e7 (run with --ion -n), tested on 64 bit: var SECTION = "15.5.4.7-2"; (SECTION, "var s = new String('hello'); s.lastIndexOf('ll', 4)", ("hello", "ll", 4), "var s = new String('hello'); s.lastIndexOf('ll', 4)")(SECTION, "var s = new String('hello'); s.lastIndexOf('ll', 5)", LastIndexOf("hello", "ll", 5), "var s = new String('hello'); s.lastIndexOf('ll', 5)")(SECTION, "var s = new String('hello'); s.lastIndexOf('ll', 6)", ("hello", "ll", 6), "var s = new String('hello'); s.lastIndexOf('ll', 6)"); function LastIndexOf(string, search, position) { for (;;) for (;;) if (string != search) break; }
The bug here appears to be that alias analysis wants to visit inner backedges before outer ones, which is an assumption we also make in the greedy allocator (FindNaturalLoops). I'm not sure whether this is a bad assumption on our part or whether there is an easy fix to block re-ordering.
Assignee | ||
Comment 2•12 years ago
|
||
(In reply to David Anderson [:dvander] from comment #1) > The bug here appears to be that alias analysis wants to visit inner > backedges before outer ones, which is an assumption we also make in the > greedy allocator (FindNaturalLoops). I'm not sure whether this is a bad > assumption on our part or whether there is an easy fix to block re-ordering. I'd like to maintain this invariant; it simplifies loop-based analyses and may avoid some correctness issues. We should try to fix block re-ordering so that loop blocks are contiguous, this will also allow us to remove some code to handle non-contiguous loops from LSRA. I have a WIP patch that seems to work, but I hit some unrelated problems along the way, going to fix these first.
Assignee: general → jdemooij
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•12 years ago
|
||
We weren't compiling labeled breaks since I added JSOP_LABEL (bug 684384).
Attachment #596664 -
Flags: review?(dvander)
Updated•12 years ago
|
Attachment #596664 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 4•12 years ago
|
||
Fix the loop depths for blocks from inlined functions, "broken" loops and some other cases. An alternative is to add a pass to calculate loop depths (after critical edge splitting), I can do that in a follow-up bug if you want.
Attachment #596971 -
Flags: review?(dvander)
Assignee | ||
Comment 5•12 years ago
|
||
Fix an off-by-one error.
Attachment #596971 -
Attachment is obsolete: true
Attachment #596971 -
Flags: review?(dvander)
Attachment #596992 -
Flags: review?(dvander)
Assignee | ||
Comment 6•12 years ago
|
||
Changes ReorderBlocks to not insert any loop successor blocks between the loop header and backedge. These blocks are added to a vector and inserted after the backedge.
Attachment #596993 -
Flags: review?(dvander)
Assignee | ||
Updated•12 years ago
|
Attachment #596993 -
Attachment is patch: true
Comment on attachment 596992 [details] [diff] [review] Part 2: Fix some loop depth problems Review of attachment 596992 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/IonBuilder.cpp @@ +1003,5 @@ > + JS_ASSERT(loopDepth_); > + loopDepth_--; > + > + // A broken loop is not a real loop (it has no header or backedge), so > + // reset the loop depth. What's the danger if we just lied about the loop depth for these blocks?
Attachment #596992 -
Flags: review?(dvander) → review+
Updated•12 years ago
|
Attachment #596993 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 8•12 years ago
|
||
https://hg.mozilla.org/projects/ionmonkey/rev/4f9b25f60991 https://hg.mozilla.org/projects/ionmonkey/rev/435100cc71d1 https://hg.mozilla.org/projects/ionmonkey/rev/a1500e708273 (In reply to David Anderson [:dvander] from comment #7) > Comment on attachment 596992 [details] [diff] [review] > Part 2: Fix some loop depth problems > > What's the danger if we just lied about the loop depth for these blocks? IIRC, there was no real danger but it broke some invariants I wanted to assert.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 9•11 years ago
|
||
A testcase for this bug was automatically identified at js/src/jit-test/tests/ion/bug724975.js.
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•