Support scalar replacement of arguments when used in spread calls
Categories
(Core :: JavaScript Engine: JIT, enhancement, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox96 | --- | fixed |
People
(Reporter: anba, Assigned: anba)
References
(Blocks 1 open bug)
Details
Attachments
(4 files)
The TypeScript compiler uses super(...arguments) to implement default derived class constructors. For example:
class Example extends Object {
public name = "field";
}
Is compiled to:
class Example extends Object {
constructor() {
super(...arguments);
this.name = "field";
}
}
Right now we don't support this pattern, which leads to creating the arguments object and additionally an array object for the spread call.
|
Assignee | |
Comment 1•3 years ago
|
||
The current JSOp::OptimizeSpreadCall optimisation only works when the input
is already a packed array. So it won't help to optimise code like
super(...arguments), which is emitted by the TypeScript compiler when
creating default derived class constructors.
For example:
class Example extends Object {
public name = "field";
}
Is compiled to:
class Example extends Object {
constructor() {
super(...arguments);
this.name = "field";
}
}
To support this pattern, JSOp::OptimizeSpreadCall is changed to return a
packed array instead of returning true when the optimisation can be applied.
When the optimisation can't be used, undefined is returned instead of false.
The emitted byte code looks roughly like:
array_or_undef = optimiseSpreadCall(value);
if (array_or_undef === undefined) {
// Emit iterator protocol code.
}
spreadCall(array_or_undef);
The additional comparison array_or_undef === undefined needs to be handled
during scalar replacement, so we can still scalar replace arrays in optimised
spread calls.
Parts 2-4 will add arguments support to JSOp::OptimizeSpreadCall.
|
|
Assignee | |
Comment 2•3 years ago
|
||
When the arguments object is still in its initial state (no overridden
elements, length, or iterator), ...arguments can be optimised to directly
create the spread array from the arguments' content instead of going through
the iterator code.
The CacheIR code additionally disallows overridden or forwarded arguments in
order to further optimise this operation during scalar replacement. See part 4.
Depends on D130986
|
|
Assignee | |
Comment 3•3 years ago
|
||
The transpiled code simply calls a VM function and performs no further
optimisations.
Depends on D130987
|
|
Assignee | |
Comment 4•3 years ago
|
||
...arguments can be seen as if a rest array object had been created for a
function without additional formals:
// |f| and |g| have identical semantics when all iteration related functions
// are still in their initial state.
function f() { h(...arguments); }
function g(...rest) { h(...rest); }
The idea when replacing MArrayFromArgumentsObject is now as follows:
We replace MArrayFromArgumentsObject with the identical instructions we use
when creating a rest array (cf. WarpBuilder::build_Rest()), except that we set
the number of formals to zero. So inlined arguments are replaced with either
MNewArrayObject or MNewArray and non-inlined arguments are replaced with
MRest.
We then rely on scalar replacement for these replacements, so that we eventually
end up with all instructions replaced. This works because scalar replacement
processes the graph in reverse post-order and because the new instructions are
placed after MCreate{Inlined}ArgumentsObject.
The ArrayFromArgumentsObjectResult CacheIR op is changed to hold the shape of
the default array shape. This change is needed so that scalar replacement can
replace MArrayFromArgumentsObject with MNewArrayObject for the case of
inlined arguments.
Depends on D130988
|
||
Updated•3 years ago
|
Pushed by andre.bargull@gmail.com: https://hg.mozilla.org/integration/autoland/rev/1b951bd25193 Part 1: Change OptimizeSpreadCall to return the optimised array or undefined. r=iain https://hg.mozilla.org/integration/autoland/rev/b89529d42aad Part 2: Handle arguments in optimised spread calls. r=iain https://hg.mozilla.org/integration/autoland/rev/51f6dd19dc14 Part 3: Transpile ArrayFromArgumentsObjectResult. r=iain https://hg.mozilla.org/integration/autoland/rev/6ee89e252b76 Part 4: Scalar replace MArrayFromArgumentsObject. r=iain
|
||
Comment 6•2 years ago
|
||
Backed out 6 changesets (Bug 1741411, Bug 1740737) for causing build bustages on CacheIR.cpp.
Backout link
Push with failures
Failure Log
|
|
Assignee | |
Updated•2 years ago
|
|
|
Assignee | |
Comment 7•2 years ago
|
||
Rooting issue in part 4, I've got a fix, just testing in on try to be sure I didn't miss anything else.
Pushed by andre.bargull@gmail.com: https://hg.mozilla.org/integration/autoland/rev/315753e83f1a Part 1: Change OptimizeSpreadCall to return the optimised array or undefined. r=iain https://hg.mozilla.org/integration/autoland/rev/3f52ee28f927 Part 2: Handle arguments in optimised spread calls. r=iain https://hg.mozilla.org/integration/autoland/rev/f08c45b5d573 Part 3: Transpile ArrayFromArgumentsObjectResult. r=iain https://hg.mozilla.org/integration/autoland/rev/11c51e909cff Part 4: Scalar replace MArrayFromArgumentsObject. r=iain
|
||
Comment 9•2 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/315753e83f1a
https://hg.mozilla.org/mozilla-central/rev/3f52ee28f927
https://hg.mozilla.org/mozilla-central/rev/f08c45b5d573
https://hg.mozilla.org/mozilla-central/rev/11c51e909cff
Description
•