Closed
Bug 722331
Opened 12 years ago
Closed 12 years ago
IonMonkey: Bailing out when unboxing OSR values
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: jandem, Assigned: jandem)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
5.29 KB,
patch
|
dvander
:
review+
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
Consider this micro-benchmark (basic-fpops from AWFY assorted). -- function test_add(x, y) { var z = 0; for (var i = 0; i < x; i++) z += y; return z; } var t = new Date; test_add(0x3ffffff, 10.5); print(new Date - t); -- |z| is a double at the loop header, but is unboxed as int32, since it has type int32 at the predecessor. To fix this, we can either: 1) Unbox based on TypeScript::SlotTypes. This works fine for the micro-benchmark but is not as precise as we want, since it does not operate on SSA values. 2) Use the "newValues" analysis info (a list of SSAValue's which have different types at the join point). 3) Not eagerly unboxing OSR values for which the predecessor type != the known slot type. I'd like to try 2) first. Brian, do you think this is okay/safe? This problem affects SS partial-sums and crypto-sha1.
Comment 1•12 years ago
|
||
Sure, 2) should work fine.
Assignee | ||
Comment 2•12 years ago
|
||
Passes tests. The micro-benchmark in comment 0 drops from 3100 ms to 230 ms and some SS tests are a bit faster (most notably partial-sums, 22 ms to 14 ms). Comment 0 was wrong about crypto-sha1, I think that's another problem after all.
Attachment #592856 -
Flags: review?(dvander)
Attachment #592856 -
Flags: review?(bhackett1024)
Comment 3•12 years ago
|
||
Comment on attachment 592856 [details] [diff] [review] Patch Review of attachment 592856 [details] [diff] [review]: ----------------------------------------------------------------- If I had to guess, since crypto-sha1 regressed with bug 719541 the problem there is probably that we hoist a bounds check badly and take bailouts because of that. JM+TI solves this by tracking when it invalidates code due to a failed hoisted check, and after recompiling it does not do hoisting for the involved script. IM will need something similar.
Attachment #592856 -
Flags: review?(bhackett1024) → review+
Updated•12 years ago
|
Attachment #592856 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 4•12 years ago
|
||
https://hg.mozilla.org/projects/ionmonkey/rev/990ee31c370a I'll file a follow-up bug for the LICM issue in comment 3.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•