Warp: fix code TODOs
Categories
(Core :: JavaScript Engine: JIT, task, P2)
Tracking
()
People
(Reporter: jandem, Assigned: jandem)
References
(Blocks 1 open bug)
Details
Attachments
(7 files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
Now that we're more feature complete, we should start addressing the remaining TODO comments.
Assignee | ||
Comment 1•4 years ago
|
||
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.
Assignee | ||
Comment 2•4 years ago
|
||
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
Assignee | ||
Comment 3•4 years ago
|
||
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
Assignee | ||
Comment 4•4 years ago
|
||
Depends on D88962
Assignee | ||
Comment 5•4 years ago
|
||
Longer-term more CompileInfo fields should be moved into WarpSnapshot, we can
then dump these fields.
Depends on D88963
Assignee | ||
Comment 6•4 years ago
|
||
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
Assignee | ||
Comment 7•4 years ago
|
||
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.
Assignee | ||
Updated•4 years ago
|
Comment 9•4 years ago
|
||
bugherder |
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
Assignee | ||
Comment 11•4 years ago
|
||
(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.
Updated•4 years ago
|
Description
•