1692855 - Warp can't elide MLexicalCheck

Status: NEW

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

Description

[André Bargull [:anba]]

Test case from bug 1362930:

function captureVar(){
    var itr = 0;
    function f(){ ++itr; }

    let start = dateNow();
    for(let i = 0; i < 50000000; ++i){
        f();
    }
    print("capture var:", dateNow() - start);
}
captureVar();

function captureLet(){
    let itr = 0;
    function f(){ ++itr; }

    let start = dateNow();
    for(let i = 0; i < 50000000; ++i){
        f();
    }
    print("capture let:", dateNow() - start);
}
captureLet();

Expected: captureLet takes the same amount of time as captureVar.
Actual: captureLet is more than twice as slow as captureVar.

---

For the var case, Warp folds the load and the unbox into a single MLoadFixedSlotAndUnbox, whereas for the let case Warp emits the sequence LoadFixedSlot, MLexicalCheck, and finally MUnbox. The additional MLexicalCheck is emitted because neither the frontend nor Warp are currently able to prove that the JSOp::CheckAliasedLexical isn't actually needed.

IonBuilder was able to elide the MLexicalCheck when the type-set didn't include JS_UNINITIALIZED_LEXICAL.

Comment 1

[André Bargull [:anba]]

I've briefly looked into this: We could omit the MLexicalCheck in the « LoadFixedSlot, MLexicalCheck, MUnbox » sequence by relying on that uninitialised variables are represented by JS_UNINITIALIZED_LEXICAL, so when we also use MLoadFixedSlotAndUnbox for that sequence, we should get a bailout because the value doesn't have the expected type. The downside is that we then can't differentiate between an unexpected type and an uninitialised lexical when we handle the UnboxFolding bailout kind from MLoadFixedSlotAndUnbox. But apart from that, we'd get the same performance numbers irrespective whether var or let is used.

Comment 2

[Iain Ireland [:iain]]

Another option would be to set the bailout kind to UninitializedLexical when folding LoadFixedSlot/MLexicalCheck/MUnbox to MLoadFixedSlotAndUnbox, and skip folding MLexicalCheck if we've ever failed a lexical check.'

Note that it's already the case that we can get an UnboxFolding bailout in cases where we would have hit the unbox anyway, and we don't worry about it too much. We have special machinery to make sure we don't turn LICM off unless we're sure it's causing problems, because LICM is often a big win, but I think FoldLoadsWithUnbox is generally only going to give an incremental improvement (except in microbenchmarks like this one where it's the only thing happening), so it seems better to limit the number of times we recompile.

Comment 3

[André Bargull [:anba]]

But if we set the bailout kind to UninitializedLexical and then get the wrong type, for example String instead of Int32, won't that lead to setting failedLexicalCheck on the script, which means we won't be able to move any lexical checks anymore?

Comment 4

[Iain Ireland [:iain]]

Yeah, in retrospect the second half of my comment kind of supersedes the first. It is another option, but it's maybe not a very good one. :P

I think your initial suggestion is good.