Specialize calls to native functions with variadic parameters
Categories
(Core :: JavaScript Engine: JIT, enhancement, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox126 | --- | fixed |
People
(Reporter: anba, Assigned: anba)
References
(Blocks 1 open bug)
Details
Attachments
(6 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
The existing LApplyArgsGeneric
and friends instructions push the arguments in preparation for a JIT to JIT call, which means when calling a native function, we first have reorder the arguments in jit::InvokeFunction
before calling the native function. This is slow when the number of arguments exceeds in inline stack space of GenericArgsBase::v_
, i.e. there are more than six arguments, because we first have to heap allocate enough space to hold the arguments.
Assignee | ||
Comment 1•2 months ago
|
||
Part 3 will also call this new method.
Assignee | ||
Comment 2•2 months ago
|
||
Move arguments length checks into separate methods for reusability. See part 3.
Depends on D206351
Assignee | ||
Comment 3•2 months ago
|
||
The existing LApplyArgsGeneric
and friends instructions push the arguments in
preparation for a JIT to JIT call, which means when calling a native function,
we first have reorder the arguments in jit::InvokeFunction
before calling the
native function. This is slow when the number of arguments exceeds in inline
stack space of GenericArgsBase::v_
, i.e. there are more than six arguments,
because we first have to heap allocate enough space to hold the arguments.
This commit adds specializations for LApplyArgsGeneric
and other instructions
when the target function is known to be a native function. These new instructions
push the arguments in the correct order for JIT to C++ calls, so we no longer
have to reorder the arguments.
Depends on D206352
Assignee | ||
Comment 4•2 months ago
|
||
This extra move was necessary when we still computed the "extra stack space" and
CodeGenerator::emitCallInvokeFunction
contained this code:
// Push the space used by the arguments.
masm.moveStackPtrTo(objreg);
masm.Push(extraStackSize);
This code has been removed in bug 1773971.
Depends on D206353
Assignee | ||
Comment 5•2 months ago
|
||
CodeGenerator.cpp:
- Improve comment why we check for odd/even number of arguments.
- Reformat comments which predate the change to two-space indent.
- Improve comment for
extraFormals
. - Remove comments about "extra stack space". (Extra stack space was removed in bug 1774390.)
- Remove "Initially the number of args to the caller." comment, because it's not
true for scalar replaced rest-parameters. And "Initially" makes it sound like that
the register value is changed at some point, but that's actually not the case.
LIR-shared.h:
- Align case for
tmpObjReg
andtmpCopy
.
Lowering.cpp
- Rename "stack counter register" to "copy register" to match LIR instruction use.
Depends on D206354
Assignee | ||
Comment 6•2 months ago
|
||
It was useful to use an explicit parameter when scratch
was used to hold the
extra stack space. But now that we no longer have to compute the extra stack
space, it's no longer necessary to have an out-param and we can instead use a
local variable similar to how ToRegister(apply->getTempObject())
is used.
Depends on D206355
Updated•2 months ago
|
Pushed by andre.bargull@gmail.com: https://hg.mozilla.org/integration/autoland/rev/d3e12936814d Part 1: Add infallible MacroAssembler::loadArgumentsObjectLength. r=jandem https://hg.mozilla.org/integration/autoland/rev/39575eed5f8a Part 2: Add CodeGenerator::emitApply{Args,ArgsObject,Array}Guard. r=jandem https://hg.mozilla.org/integration/autoland/rev/3ef1b0ae5ce0 Part 3: Specialize calls to native functions with variadic parameters. r=jandem https://hg.mozilla.org/integration/autoland/rev/2ea52b74deea Part 4: Remove extra stack pointer move. r=jandem https://hg.mozilla.org/integration/autoland/rev/7351cc1e72dd Part 5: Update comments and variable names. r=jandem https://hg.mozilla.org/integration/autoland/rev/7a8990de860d Part 6: Remove "scratch" register parameter from emitPushArguments. r=jandem
Comment 8•2 months ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d3e12936814d
https://hg.mozilla.org/mozilla-central/rev/39575eed5f8a
https://hg.mozilla.org/mozilla-central/rev/3ef1b0ae5ce0
https://hg.mozilla.org/mozilla-central/rev/2ea52b74deea
https://hg.mozilla.org/mozilla-central/rev/7351cc1e72dd
https://hg.mozilla.org/mozilla-central/rev/7a8990de860d
Description
•