Closed Bug 1619343 Opened 8 months ago Closed 8 months ago

Support SpreadNew and SpreadSuperCall in Ion

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla75
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!

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|.

Split |emitPushArguments()| so we can reuse the elements copying code for
part 4.

Depends on D64978

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

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

Blocks: es6perf
Priority: -- → P1
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
You need to log in before you can comment on or make changes to this bug.