Closed
Bug 595336
Opened 14 years ago
Closed 9 years ago
Fix for bug 590083 causes ARM crashes
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
INCOMPLETE
People
(Reporter: sayrer, Unassigned)
References
Details
(Keywords: crash)
Attachments
(1 file)
1.75 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
We can fix this by disabling ARM JIT for the moment, but we do have to fix this.
Reporter | ||
Updated•14 years ago
|
blocking2.0: --- → betaN+
Updated•14 years ago
|
tracking-fennec: --- → 2.0b1+
Updated•14 years ago
|
tracking-fennec: 2.0b1+ → ---
Updated•14 years ago
|
tracking-fennec: --- → ?
Updated•14 years ago
|
tracking-fennec: ? → 2.0b1+
Reporter | ||
Comment 1•14 years ago
|
||
I disabled the ARM method jit for the moment http://hg.mozilla.org/tracemonkey/rev/3a6f15524d13
Comment 4•14 years ago
|
||
The assertion failures on ARM look remarkably similar to the failure in bug 595470, so this may be the same issue.
Comment 6•14 years ago
|
||
Increasing the size of the offset fields in CallICInfo fixes this crash. Is 8 bits really not enough to describe the offset? Clearly it's not. MICs weren't that big when I last looked. 8 bits gives enough space for 64 ARM instructions. I increased these fields to 16 bits each to pass the tests, but I didn't check the optimal size. It's worth mentioning that on ARM, the offsets will always be 4-byte-aligned, so if we wrap the assignment (and check) in a macro or inline function, we should be able to save 2 bits on each field. Is it really worth storing offsets like this, rather than using CodeLocationLabel objects? Yes, it makes the structure smaller, but is it a worthwhile optimization? There surely aren't hundreds of these kicking around, and if there are, many will be identical anyway, so we could alias them from within IC records. (This code still feels fragile to me.)
Comment 7•14 years ago
|
||
There is one of these for every callsite, so hundreds is probably a good estimate. Even with a hundred thousand though, a few extra bytes won't matter. If CodeLocationLabel makes things much easier, we can do that. In the early JM days, large offsets have pointed to bugs or inefficiencies in FrameState. But those are all gone now.
Reporter | ||
Comment 9•14 years ago
|
||
Can we land the trivial fix to get ARM working again?
Comment on attachment 474700 [details] [diff] [review] A trivial (but not ideal) fix. r=me on this fix but we should have a follow-up, to either minimize the fields or use CodeLocationLabel.
Attachment #474700 -
Flags: review+
Reporter | ||
Comment 11•14 years ago
|
||
I landed this http://hg.mozilla.org/tracemonkey/rev/5899a40b6d6d
Updated•14 years ago
|
Severity: normal → critical
Comment 13•14 years ago
|
||
(In reply to comment #12) > blocking 2.0 final for real fix I don't think it needs to block 2.0.
OS: Mac OS X → All
Hardware: x86 → ARM
Reporter | ||
Updated•14 years ago
|
blocking2.0: betaN+ → -
Updated•14 years ago
|
tracking-fennec: 2.0+ → 2.0-
Comment 14•13 years ago
|
||
robert, is there anything left to do here?
Updated•13 years ago
|
OS: All → Android
Assignee | ||
Updated•10 years ago
|
Assignee: general → nobody
Comment 15•9 years ago
|
||
Pertains to obsolete compilers (TM, JM).
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → INCOMPLETE
You need to log in
before you can comment on or make changes to this bug.
Description
•