Closed Bug 722331 Opened 12 years ago Closed 12 years ago

IonMonkey: Bailing out when unboxing OSR values

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Sure, 2) should work fine.
Attached patch PatchSplinter Review
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 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+
Attachment #592856 - Flags: review?(dvander) → review+
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.