Closed Bug 946478 Opened 6 years ago Closed 6 years ago

Crash [@ js::jit::BacktrackingAllocator::addLiveInterval] or Assertion failure: !minimalInterval(interval), at jit/BacktrackingAllocator.cpp

Categories

(Core :: JavaScript Engine: JIT, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: gkw, Assigned: sunfish)

References

(Blocks 1 open bug)

Details

(4 keywords)

Crash Data

Attachments

(3 files, 1 obsolete file)

Attached file lldb stack
function f(x) {
    x ? d() : (y ? (x | 0) % (x | 0) : y)
}
for (var j = 0; j < 2; ++j) {
    try {
        f(!(0(f())))
    } catch (e) {}
}

asserts js debug shell on m-c changeset 526e12792fc8 with --ion-eager --ion-gvn=off --ion-regalloc=backtracking at Assertion failure: !minimalInterval(interval), at jit/BacktrackingAllocator.cpp

My configure flags are:

CC="clang -Qunused-arguments" AR=ar CXX="clang++ -Qunused-arguments" sh ./configure --target=x86_64-apple-darwin12.5.0 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --with-ccache --disable-threadsafe

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/6787bcb8ea7e
user:        Dan Gohman
date:        Mon Dec 02 13:41:54 2013 -0800
summary:     Bug 944963 - IonMonkey: Add a ModSelf operator to fix an x86 constraint problem with x%x. r=bhackett

Dan, is bug 944963 a likely regressor?
Flags: needinfo?(sunfish)
It looks like the LDivSelf/LModSelf trick is not going to work.

This testcase shows a case where Lowering's check for mod->rhs() == mod->lhs() returns false, since the operands are separate MTruncateToInt32 operators, however the register allocator is managing to coalesce these operators away and use the same register for both operands, re-exposing the original x86 div constraint puzzle that LDivSelf/LModSelf were supposed to be preventing.

A new approach will be needed.
Assignee: general → sunfish
Flags: needinfo?(sunfish)
Could the check during lowering be done better here?  Where is the coalescing happening?
(In reply to Brian Hackett (:bhackett) from comment #2)
> Could the check during lowering be done better here?  Where is the
> coalescing happening?

In LIRGenerator::visitTruncateToInt32, the Int32 case just does a redefine, which reuses the virtual register.

Theoretically the Lowering code could also look for TruncateToInt32 and ToInt32 and maybe TypeBarrier and anything else which could end up being just a redefine, but that seems fragile.
Attached file Opt stack
This seems to crash opt too.
Crash Signature: [@ js::jit::BacktrackingAllocator::addLiveInterval]
Keywords: crash
Summary: Assertion failure: !minimalInterval(interval), at jit/BacktrackingAllocator.cpp → Crash [@ js::jit::BacktrackingAllocator::addLiveInterval] or Assertion failure: !minimalInterval(interval), at jit/BacktrackingAllocator.cpp
QA Contact: general
I plan to fix this by changing the lowering for divide/mod to use a copy to get the input into the required register, rather than trying to describe this requirement to the register allocator. This is slightly suboptimal, but it's simple, and it's probably good enough most of the time, given that there's already a divide/mod happening.
Attached patch divmod-use-copies.patch (obsolete) — Splinter Review
This patch implements the approach of just using copies to get the div lhs into eax. Slightly less optimal, but much simpler.
Attachment #8374462 - Flags: review?(bhackett1024)
Attachment #8374462 - Flags: review?(bhackett1024) → review+
Keywords: checkin-needed
Updating patch to add reviewer.
Attachment #8374462 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/ad545722ca5f
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Discussed offline too, but commenting here so it doesn't get forgotten - it looks like this regressed asm-corrections by 5% and asm-copy by 4%, both on no-asmjs. Interestingly, primes sped up by 6% at the same time.

Might be nothing but noise I guess, but perhaps worth verifying.

Full changelog where the difference happens is

http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d2c4ae312b66&tochange=dc46d95c4787

, the others do not look relevant.
You need to log in before you can comment on or make changes to this bug.