Open Bug 1692856 Opened 4 years ago Updated 4 years ago

Transition from GuardSpecificFunction to GuardFunctionScript breaks trial-inlining

Categories

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

defect

Tracking

()

People

(Reporter: anba, Unassigned)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

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);
}
for (var i = 0; i < 5; ++i) captureVar();

Expected: f is always inlined.
Actual: f is only inlined in the first call to captureVar.


In the first call to captureVar, the inner function f is inlined using the GuardSpecificFunction guard. When captureVar is called a second time, GuardSpecificFunction fails and is then replaced by GuardFunctionScript.

But because f was already inlined once, AttachBaselineCacheIRStub() marks it as non-inlinable in https://searchfox.org/mozilla-central/rev/8cb90829ccf18f95cd75d8419c84729c754a8e83/js/src/jit/BaselineCacheIRCompiler.cpp#2089-2091. TrialInliner::maybeSingleStub() will now never consider it as inlinable again https://searchfox.org/mozilla-central/rev/8cb90829ccf18f95cd75d8419c84729c754a8e83/js/src/jit/TrialInlining.cpp#146-148.

This could be a regression from bug 1664786 (https://phabricator.services.mozilla.com/D90410).

Bug 1662688 tried to support this but D90410 then broke it again... We disable trial inlining for an IC if it succeeded once and we then attach a new stub. I wonder how bad it would be to not disable in that case.

(This also comes back to the old problem of not having a good way to track "what did we try before" for ICs/CacheIR right now...)

Severity: -- → S3
Priority: -- → P2
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.