1662366 - Warp: fix code TODOs

Status: RESOLVED FIXED

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

Description

[Jan de Mooij [:jandem]]

Now that we're more feature complete, we should start addressing the remaining TODO comments.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1662366 part 1 - Remove some TODOs in WarpOracle. r?iain!

The ones in maybeInlineIC have been addressed. The one about extra var environments
isn't really a TODO for Warp specifically.

Also change the stub data copying to just use nullptr if there's no stub data.
This is fairly common for simpler IC stubs.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1662366 part 2 - Call setImplicitlyUsedUnchecked for JSOp::ToString. r?iain!

Allowlisting this op in the checker is wrong because we don't want a
MagicValue to show up for JSOp::ToString in Baseline.

Depends on D88960

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1662366 part 3 - Remove/rename some TODOs in WarpBuilder. r?iain!

Use TODO(post-Warp) for TODOs that can only be addressed when IonBuilder is gone.

Remove some TODOs that are no longer relevant or don't need to be addressed
short-term.

Depends on D88961

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1662366 part 4 - Rename some TODOs to TODO(post-Warp). r?iain!

Depends on D88962

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1662366 part 5 - Print CompileInfo pointer in WarpCacheIR::dumpData. r?iain!

Longer-term more CompileInfo fields should be moved into WarpSnapshot, we can
then dump these fields.

Depends on D88963

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1662366 part 6 - Trace cloned CacheIR stub data. r?iain!

Initially the plan was to reuse TraceCacheIRStub but this patch adds the tracing
code for Warp separately because:

• CacheIR stub data in Warp contains nursery indexes. It's nice to keep that out of the IC-tracing code.
• We can use WarpGCPtr similar to other snapshotted GC pointers. It asserts GC things are not moved.

Depends on D88964

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1662366 part 7 - Some MBoxNonStrictThis optimizations. r?anba!

This adds an OOL path to handle null/undefined => globalThis without a VM call.
IonBuilder optimizes that case based on TI and this ensures we're not a lot slower
for that.

Also give the MIR instruction an empty AliasSet so we can optimize it better in
inlined functions. We no longer have the thisValue hook mentioned in the comment.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/895c8b18a46b
part 1 - Remove some TODOs in WarpOracle. r=iain
https://hg.mozilla.org/integration/autoland/rev/26c1c9e63455
part 2 - Call setImplicitlyUsedUnchecked for JSOp::ToString. r=iain
https://hg.mozilla.org/integration/autoland/rev/79068e7fabea
part 3 - Remove/rename some TODOs in WarpBuilder. r=iain
https://hg.mozilla.org/integration/autoland/rev/3b77c688e440
part 4 - Rename some TODOs to TODO(post-Warp). r=iain
https://hg.mozilla.org/integration/autoland/rev/4d27ace84a3e
part 5 - Print CompileInfo pointer in WarpCacheIR::dumpData. r=iain
https://hg.mozilla.org/integration/autoland/rev/4259f11d48a1
part 6 - Trace cloned CacheIR stub data. r=iain
https://hg.mozilla.org/integration/autoland/rev/237e445d595e
part 7 - Some MBoxNonStrictThis optimizations. r=anba

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/895c8b18a46b
https://hg.mozilla.org/mozilla-central/rev/26c1c9e63455
https://hg.mozilla.org/mozilla-central/rev/79068e7fabea
https://hg.mozilla.org/mozilla-central/rev/3b77c688e440
https://hg.mozilla.org/mozilla-central/rev/4d27ace84a3e
https://hg.mozilla.org/mozilla-central/rev/4259f11d48a1
https://hg.mozilla.org/mozilla-central/rev/237e445d595e

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Does this still need to be open?

Comment 11

[Jan de Mooij [:jandem]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #10)

Does this still need to be open?

There's more to do in a few weeks after we remove IonBuilder/TI, but we can do that in other bugs to simplify release tracking.