1838554 - Use an IC for JSOp::BuiltinObject

Status: RESOLVED FIXED

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

Description

[Iain Ireland [:iain]]

We currently handle BuiltinObject in the baseline interpreter by generating a VM call to BuiltinObjectOperation, and in the baseline compiler by calling BuiltinObjectOperation during compilation to get the object, and then baking it in directly.

The interpreter implementation is slow. The compiler implementation doesn't play nicely with Bryan's work to share selfhosted baseline code, and also doesn't play nicely with my work to do offthread baseline compiles.

We can solve all these problems at once by using a baseline IC, much like we already do for GetIntrinsic.

Comment 1

[Iain Ireland [:iain]]

Attachment: WIP: Bug 1838554 - Use an IC for JSOp::BuiltinObject

Comment 2

[Iain Ireland [:iain]]

Attachment: Bug 1838554: Rename GetIntrinsic ICKind to LazyConstant r=jandem

A GetIntrinsic IC will always produce the same result, so it looks up the result in the fallback stub and then attaches a trivial stub that returns the correct value. We can reuse the same idea for JSOp::BuiltinObject and JSOp::ImportMeta. This makes blinterp more efficient (because we only have to do the VM call to look up the result once), and works better for offthread baseline compilation (where we can't do the VM call inside the compiler and bake in the result).

In preparation, this patch renames the ICKind. The next two patches will update the ops.

Comment 3

[Iain Ireland [:iain]]

Attachment: Bug 1838554: Use an IC for JSOp::BuiltinObject r=jandem

Comment 4

[Iain Ireland [:iain]]

Attachment: Bug 1838554: Use an IC for JSOp::ImportMeta r=jandem

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/007ce442c413
Rename GetIntrinsic ICKind to LazyConstant r=jandem
https://hg.mozilla.org/integration/autoland/rev/9eeaab44d7d2
Use an IC for JSOp::BuiltinObject r=jandem
https://hg.mozilla.org/integration/autoland/rev/8cde9d09e842
Use an IC for JSOp::ImportMeta r=jandem

Comment 6

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/007ce442c413
https://hg.mozilla.org/mozilla-central/rev/9eeaab44d7d2
https://hg.mozilla.org/mozilla-central/rev/8cde9d09e842

Comment 7

[Mayank Bansal]

Between this bug and some pieces of bug 1930281, there are regressions on Jetstream2 on Windows, Linux and MacOS

12.3% on n-body-sp
6% on Richards-wasm
4.8% on crypto-aes
5% on date-formatxparb
9% on delta-blue-average

Comment 8

[Jan de Mooij [:jandem]]

Could be the compilerWarmUpThreshold refactoring, although I don't see any issues with it looking at the patch again.

Some of these numbers are stable enough that we should be able to narrow it down with a few try pushes.

Comment 9

[Mayank Bansal]

(there may be some improvements on other sub-tests including SP3 too . Not validated yet by perf-sheriffs yet, so consider my comment as a unverified rumour)