Closed Bug 1664357 Opened 3 months ago Closed 2 months ago

Warp: Inline constructors

Categories

(Core :: JavaScript Engine: JIT, task, P1)

task

Tracking

()

RESOLVED FIXED
82 Branch
Tracking Status
firefox82 --- fixed

People

(Reporter: iain, Assigned: iain)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

This turns out to be relatively easy.

Instead of storing a flag in GuardSpecificFunction, I reused the CallFlags that are already being stored in the call. This also lets us avoid a VM call in the baseline IC.

After Ion is gone, we can consider storing a ConstructorKind in CallFlags (with values None, UninitializedThis, Template, and VMCall) and moving the template object offset from MetaTwoByte into CallScriptedFunction.

We already did the hard work to handle this creation for transpiling, so inlining is pretty straight-forward.

patchInlinedReturn needs to know whether we're calling a derived constructor to avoid repeated bailouts in MReturnFromCtor. I used a similar solution to what anba did here: https://phabricator.services.mozilla.com/D58785). Once Ion is gone, we could consider modifying MReturnFromCtor instead.

Depends on D89872

AFAICT, outside of the bytecode generator, there is no longer any difference between JSOp::Super and JSOp::New except for the expression decompiler.

Depends on D89873

To make sure we're testing recursive inlining, I've tweaked the thresholds to ensure that we can inline two levels deep without any loops. I've also made the inlining heuristics slightly more conservative, which prevents check-earley-boyer from timing out.

I've also included a few drive-by fixes. The change to isRecursive in DoTrialInlining only affects the jitspew message, because both ways of finding the InliningRoot are equivalent for the non-recursive case.

Depends on D89874

Blocks: 1664573

Rewriting this code to make it obvious that we only attach specialized constructor stubs if we have enough information about this to inline.

Note: the old code used skipAttach in more cases, but after looking at it more closely, the new script analysis is the only case where it makes sense.

This version matches the corresponding pre-CacheIR call IC code (https://searchfox.org/mozilla-central/rev/3c073ca1ae02fd52fd6be584aa343b78999842fa/js/src/jit/BaselineIC.cpp#3509-3545).

Depends on D89875

Pushed by iireland@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b7b3ae95e27d
Store needsUninitializedThis in CallFlags r=jandem
https://hg.mozilla.org/integration/autoland/rev/6c5352e5d39c
Inline JSOp::New r=jandem
https://hg.mozilla.org/integration/autoland/rev/71febd4c390a
Inline JSOp::Super r=jandem,anba
https://hg.mozilla.org/integration/autoland/rev/69b330d503c4
Tweak thresholds for --fast-warmup r=jandem
https://hg.mozilla.org/integration/autoland/rev/89b84ae80b98
Refactor GetTemplateObjectForScripted r=jandem
You need to log in before you can comment on or make changes to this bug.