Closed
Bug 875656
Opened 11 years ago
Closed 11 years ago
Assertion failure: !minimalInterval(interval), at ion/BacktrackingAllocator.cpp with enableSPSProfilingAssertions
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla27
People
(Reporter: gkw, Assigned: sunfish)
References
Details
(Keywords: assertion, testcase, Whiteboard: [fuzzblocker])
Attachments
(2 files, 1 obsolete file)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
enableSPSProfilingAssertions(false);
Object.getOwnPropertyNames(this);
asserts js debug shell on m-c changeset 53bfd38cbc8c with --ion-eager --ion-regalloc=backtracking at Assertion failure: !minimalInterval(interval), at ion/BacktrackingAllocator.cpp
This blocks fuzzing --ion-regalloc=backtracking with enableSPSProfilingAssertions
Flags: needinfo?(bhackett1024)
Reporter | ||
Updated•11 years ago
|
Summary: Assertion failure: !minimalInterval(interval), at ion/BacktrackingAllocator.cpp → Assertion failure: !minimalInterval(interval), at ion/BacktrackingAllocator.cpp with enableSPSProfilingAssertions
Reporter | ||
Comment 1•11 years ago
|
||
Brian, re-ping? This still reproduces as of m-c rev 3b955f306226.
Reporter | ||
Updated•11 years ago
|
Flags: needinfo?(kvijayan)
Comment 2•11 years ago
|
||
I'm not sure why I'm marked with needInfo on this. Is it because of the involvement of SPS enabling?
Reporter | ||
Comment 3•11 years ago
|
||
(In reply to Kannan Vijayan [:djvj] from comment #2)
> I'm not sure why I'm marked with needInfo on this. Is it because of the
> involvement of SPS enabling?
Yep, I don't know if it's SPS or the backtracking allocator at play here.
Comment 4•11 years ago
|
||
I'm pretty sure it's the backtracking allocator that's causing the problem here. SPS doesn't interact in any way with the compiler backend.
Flags: needinfo?(kvijayan)
Assignee | ||
Comment 5•11 years ago
|
||
It appears SPS does interact with the backend in one way: it makes the FramePointer unavailable to the register allocator.
x86 and x64 both have CallTempReg6 defined to rbp/ebp, which is the FramePointer. On x64 it might be possible to define use a different register, but x86 there are no other registers. With the %ebp unavailable, x86 only has 6 allocatable registers left, which is not enough for the 7 CallTempReg variables.
Flags: needinfo?(bhackett1024)
Assignee | ||
Comment 6•11 years ago
|
||
The only thing using all 7 CallTemp registers is Concat. This patch tweaks Concat's code slightly to reduce the number of registers used to 6. This reduces register pressure in general, and it also fixes this bug.
Assignee: general → sunfish
Attachment #808864 -
Flags: review?(bhackett1024)
Updated•11 years ago
|
Attachment #808864 -
Flags: review?(bhackett1024) → review+
Assignee | ||
Comment 7•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Assignee | ||
Comment 10•11 years ago
|
||
Backed out here:
https://hg.mozilla.org/integration/mozilla-inbound/rev/64222cba97fa
Reusing lhs and rhs as temporaries is not valid, because they are only declared to the register allocator as input operands. I'll need to find a different way to fix this.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 12•11 years ago
|
||
Oops, re-reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Comment 13•11 years ago
|
||
(In reply to Dan Gohman [:sunfish] from comment #10)
> Backed out here:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/64222cba97fa
>
> Reusing lhs and rhs as temporaries is not valid, because they are only
> declared to the register allocator as input operands. I'll need to find a
> different way to fix this.
This patch may also have caused bug 921263.
Assignee | ||
Comment 14•11 years ago
|
||
The useFixedAtStart utility introduced in bug 923799 may provide a way to fix the patch above.
Updated•11 years ago
|
Component: JavaScript Engine → JavaScript Engine: JIT
Reporter | ||
Updated•11 years ago
|
Target Milestone: mozilla27 → ---
Assignee | ||
Comment 15•11 years ago
|
||
This patch is similar to the previous patch, but uses useFixedAtStart to properly describe clobbering the lhs and rhs input registers.
Attachment #808864 -
Attachment is obsolete: true
Attachment #821894 -
Flags: review?(bhackett1024)
Comment 16•11 years ago
|
||
Comment on attachment 821894 [details] [diff] [review]
regalloc-concat-pressure.patch
Review of attachment 821894 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/jit/Lowering.cpp
@@ +1479,2 @@
> useFixed(lhs, CallTempReg0),
> useFixed(rhs, CallTempReg1),
I think these two should be useFixedAtStart.
Attachment #821894 -
Flags: review?(bhackett1024) → review+
Assignee | ||
Comment 17•11 years ago
|
||
(In reply to Brian Hackett (:bhackett) from comment #16)
> Comment on attachment 821894 [details] [diff] [review]
> regalloc-concat-pressure.patch
>
> Review of attachment 821894 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: js/src/jit/Lowering.cpp
> @@ +1479,2 @@
> > useFixed(lhs, CallTempReg0),
> > useFixed(rhs, CallTempReg1),
>
> I think these two should be useFixedAtStart.
Yes, fixed.
https://hg.mozilla.org/integration/mozilla-inbound/rev/f030f97fcf10
Comment 18•11 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in
before you can comment on or make changes to this bug.
Description
•