1664357 - Warp: Inline constructors

Status: RESOLVED FIXED

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

Description

[Iain Ireland [:iain]]

This turns out to be relatively easy.

Comment 1

[Iain Ireland [:iain]]

Attachment: Bug 1664357: Store needsUninitializedThis in CallFlags r=jandem

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.

Comment 2

[Iain Ireland [:iain]]

Attachment: Bug 1664357: Inline JSOp::New r=jandem

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

Comment 3

[Iain Ireland [:iain]]

Attachment: Bug 1664357: Inline JSOp::Super r=jandem

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

Comment 4

[Iain Ireland [:iain]]

Attachment: Bug 1664357: Tweak thresholds for --fast-warmup r=jandem

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

Comment 5

[Iain Ireland [:iain]]

Attachment: Bug 1664357: Refactor GetTemplateObjectForScripted r=jandem

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
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

Comment 7

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/b7b3ae95e27d
https://hg.mozilla.org/mozilla-central/rev/6c5352e5d39c
https://hg.mozilla.org/mozilla-central/rev/71febd4c390a
https://hg.mozilla.org/mozilla-central/rev/69b330d503c4
https://hg.mozilla.org/mozilla-central/rev/89b84ae80b98