Support SpreadNew and SpreadSuperCall in Ion
Categories
(Core :: JavaScript Engine: JIT, enhancement, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox75 | --- | fixed |
People
(Reporter: anba, Assigned: anba)
References
(Blocks 1 open bug)
Details
Attachments
(4 files)
It isn't too hard to support JSOp::SpreadNew and JSOp::SpreadSuperCall in Ion when we reuse the existing code for JSOp::SpreadCall. That code does have some issues, but it's all we have at the moment and we probably want to wait for WarpBuilder to write a cleaner implementation for spread calls. Also supporting JSOp::SpreadSuperCall means we're no longer forced to stay in baseline when running derived class constructors!
|
Assignee | |
Comment 1•4 years ago
|
||
Move the branch instruction out of |emitAllocateSpaceForApply()| to make it
easier to follow the control flow.
Drive-by fix:
- Reformat a comment which was broken into two lines by clang-format.
- Use the correct lir-class when accessing |ThisIndex| for |LApplyArrayGeneric|.
|
|
Assignee | |
Comment 2•4 years ago
|
||
Split |emitPushArguments()| so we can reuse the elements copying code for
part 4.
Depends on D64978
|
|
Assignee | |
Comment 3•4 years ago
|
||
SpreadNew and SpreadSuperCall will reuse this function, so prepare it to
support constructor calls by adding the missing pieces based on what we perform
in |visitCallGeneric()|.
Depends on D64979
|
|
Assignee | |
Comment 4•4 years ago
|
||
Support |JSOp::SpreadNew| and |JSOp::SpreadSuperCall| by reusing the already
present infrastructure for |JSOp::SpreadCall|.
|LConstructArrayGeneric| needs an additional register for the |new.target|
value, so we have to share |tempStackCounter| with the |new.target| register,
similar to how we already share the |elements| register with the |argc|
register.
This register sharing leads to a slightly different code in
|emitAllocateSpaceForConstructAndPushNewTarget()| compared its counterpart in
|emitAllocateSpaceForApply()|, because we have to push |new.target| onto the
stack before we have a free register available to store |extraStackSpace|. And
contrary to |emitAllocateSpaceForApply()|, we're also always pushing
|JS_ARG_POISON| when we need padding instead of leaving the stack contents
undefined. If want to squeeze out a bit more performance, we could use
masm.subFromStackPtr(Imm32(sizeof(Value))), but it probably won't make much
of a difference in practice, so it seems nicer to use the "safer" route and
always push |JS_ARG_POISON|.
Depends on D64980
|
||
Updated•4 years ago
|
Pushed by csabou@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3e88ffbc2168 Part 1: Move zero arguments jump out of emitAllocateSpaceForApply. r=jandem https://hg.mozilla.org/integration/autoland/rev/74c24cb3ac9a Part 2: Split emitPushArguments to make it reusable. r=jandem https://hg.mozilla.org/integration/autoland/rev/2f8f4267d3e7 Part 3: Prepare emitApplyGeneric to support constructor calls. r=jandem https://hg.mozilla.org/integration/autoland/rev/2872f192769e Part 4: Support SpreadNew and SpreadSuperCall in Ion. r=jandem
|
||
Comment 6•4 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/3e88ffbc2168
https://hg.mozilla.org/mozilla-central/rev/74c24cb3ac9a
https://hg.mozilla.org/mozilla-central/rev/2f8f4267d3e7
https://hg.mozilla.org/mozilla-central/rev/2872f192769e
Description
•