Inline inlinable natives through JSOp::FunCall
Categories
(Core :: JavaScript Engine: JIT, enhancement, P1)
Tracking
()
People
(Reporter: anba, Assigned: anba)
References
(Blocks 1 open bug)
Details
Attachments
(10 files, 17 obsolete 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 | |
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 |
Supporting inlining through inlinable natives when called from JSOp::FunCall
. This helps for patterns like Object.prototype.toString.call(thing)
.
Assignee | ||
Comment 1•7 months ago
|
||
Add InlinableNativeCallInfo
to hold the per function call relevant information.
The next parts of this patch stack will convert the existing functions to use
InlinableNativeCallInfo
. The last part will use InlinableNativeCallInfo
to
pass the call information from JSOp::FunCall
calls.
Assignee | ||
Comment 2•7 months ago
|
||
Replaces the accesses to argc_
, args_
, callee_
, newTarget_
, and the
CallFlags
to use InlinableNativeCallInfo
.
Depends on D129903
Assignee | ||
Comment 3•7 months ago
|
||
Replaces the accesses to argc_
, args_
, callee_
, newTarget_
, and the
CallFlags
.
Depends on D129904
Assignee | ||
Comment 4•7 months ago
|
||
Replaces the accesses to argc_
, args_
, callee_
, newTarget_
, and the
CallFlags
.
Depends on D129905
Assignee | ||
Comment 5•7 months ago
|
||
Replaces the accesses to argc_
, args_
, callee_
, newTarget_
, and the
CallFlags
.
Depends on D129906
Assignee | ||
Comment 6•7 months ago
|
||
Replaces the accesses to argc_
, args_
, callee_
, newTarget_
, and the
CallFlags
.
Depends on D129907
Assignee | ||
Comment 7•7 months ago
|
||
Replaces the accesses to argc_
, args_
, callee_
, newTarget_
, and the
CallFlags
.
Depends on D129908
Assignee | ||
Comment 8•7 months ago
|
||
Replaces the accesses to argc_
, args_
, callee_
, newTarget_
, and the
CallFlags
.
Depends on D129909
Assignee | ||
Comment 9•7 months ago
|
||
Replaces the accesses to argc_
, args_
, callee_
, newTarget_
, and the
CallFlags
.
Depends on D129910
Assignee | ||
Comment 10•7 months ago
|
||
Replaces the accesses to argc_
, args_
, callee_
, newTarget_
, and the
CallFlags
.
Depends on D129911
Assignee | ||
Comment 11•7 months ago
|
||
Replaces the accesses to argc_
, args_
, callee_
, newTarget_
, and the
CallFlags
.
Depends on D129912
Assignee | ||
Comment 12•7 months ago
|
||
Replaces the accesses to argc_
, args_
, callee_
, newTarget_
, and the
CallFlags
.
Depends on D129913
Assignee | ||
Comment 13•7 months ago
|
||
Replaces the accesses to argc_
, args_
, callee_
, newTarget_
, and the
CallFlags
.
Depends on D129914
Assignee | ||
Comment 14•7 months ago
|
||
Replaces the accesses to argc_
, args_
, callee_
, newTarget_
, and the
CallFlags
.
Depends on D129915
Assignee | ||
Comment 15•7 months ago
|
||
Replaces the accesses to argc_
, args_
, callee_
, newTarget_
, and the
CallFlags
.
Depends on D129916
Assignee | ||
Comment 16•7 months ago
|
||
Depends on D129917
Assignee | ||
Comment 17•7 months ago
|
||
To support inlining natives through JSOp::FunCall
, we now just have to fill in
InlinableNativeCallInfo
with the original this-value removed.
Depends on D129918
Assignee | ||
Comment 18•7 months ago
|
||
Hi Jan,
I've added some patches to show how we could easily inline through JSOp::FunCall
for inlinable functions. The patches are still marked as WIP, because they're lacking tests and I wanted to check with you first if you think that this approach looks sensible.
Comment 19•7 months ago
|
||
(In reply to André Bargull [:anba] from comment #18)
Hi Jan,
I've added some patches to show how we could easily inline throughJSOp::FunCall
for inlinable functions. The patches are still marked as WIP, because they're lacking tests and I wanted to check with you first if you think that this approach looks sensible.
Nice, this makes sense to me.
It would be good if argc_
, args_
, callee_
, newTarget_
were no longer accessible. What if we move tryAttachMathAbs
etc to a new class that contains the fields in InlinableNativeCallInfo
+ the writer
and some other things?
Assignee | ||
Comment 20•6 months ago
|
||
InlinableNativeIRGenerator
copies all relevant member fields from CallIRGenerator
only to keep the diff is small. The next patches will remove the pc_
, script_
,
op_
, and isFirstStub_
members.
Assignee | ||
Comment 21•6 months ago
|
||
Use the existing flags_
member field instead of recomputing CallFlags
.
This also allows to remove the pc_
member.
Depends on D132125
Assignee | ||
Comment 22•6 months ago
|
||
Change isFirstStub_
and script_
to methods which delegate to CallIRGenerator
.
Depends on D132126
Assignee | ||
Comment 23•6 months ago
|
||
When the op_
member is used to detect new
- and spread-calls, replace it
with calling CallFlags
methods. The other only remaining use is to detect
JSOp::CallIgnoresRv
. Replace that one with a method which delegates to
CallIRGenerator
.
Depends on D132127
Assignee | ||
Comment 24•6 months ago
|
||
Pass CallFlags
to fix the "TODO" note.
Depends on D132128
Assignee | ||
Comment 25•6 months ago
|
||
The callee is guaranteed to be a JSFunction
, so we can use HandleFunction
instead of HandleValue
.
Depends on D132129
Assignee | ||
Comment 26•6 months ago
|
||
emitNativeCalleeGuard()
can just read the callee from the member field instead
of receiving it as a parameter.
Depends on D132130
Assignee | ||
Comment 27•6 months ago
|
||
This parameter is mostly no longer needed. The only remaining methods which used
this parameter were:
tryAttachMathRandom()
tryAttachArrayConstructor()
tryAttachTypedArrayConstructor()
All three methods can use the callee_
member field to access the callee function.
Depends on D132131
Assignee | ||
Comment 28•6 months ago
|
||
This is in preparation for the next part, where we don't want to call
setInputOperandId()
for JSOp::FunCall
calls.
Depends on D132132
Assignee | ||
Comment 29•6 months ago
|
||
The stack layout is already in the correct format for all Function.prototype.call
calls with more than zero arguments, so we can simply call InlinableNativeIRGenerator
to inline natives. Only two adjustments were needed:
writer.setInputOperandId(0)
mustn't be called a second time.emitNativeCalleeGuard()
mustn't emit the callee guard a second time.
We also cheat a bit: Because we don't guard the callee in emitNativeCalleeGuard()
and thanks to bug 1656469, we don't have to adjust GetIndexOfArgument()
to handle
CallFlags::FunCall
.
Depends on D132133
Assignee | ||
Comment 30•6 months ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #19)
It would be good if
argc_
,args_
,callee_
,newTarget_
were no longer accessible. What if we movetryAttachMathAbs
etc to a new class that contains the fields inInlinableNativeCallInfo
+ thewriter
and some other things?
I've uploaded another patch set with the proposed design.
Updated•6 months ago
|
Updated•6 months ago
|
Assignee | ||
Comment 31•2 months ago
|
||
(In reply to André Bargull [:anba] [back from break; working through mail backlog] from comment #30)
(In reply to Jan de Mooij [:jandem] from comment #19)
It would be good if
argc_
,args_
,callee_
,newTarget_
were no longer accessible. What if we movetryAttachMathAbs
etc to a new class that contains the fields inInlinableNativeCallInfo
+ thewriter
and some other things?I've uploaded another patch set with the proposed design.
NI in case you've missed the updates.
Comment 32•2 months ago
|
||
(In reply to André Bargull [:anba] [back from break; working through mail backlog] from comment #31)
I've uploaded another patch set with the proposed design.
NI in case you've missed the updates.
I skimmed some of the patches and this looks great!
Note that JSOp::FunCall
no longer exists (bug 1760989), but I don't think that really affects these changes.
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Assignee | ||
Comment 33•2 months ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #32)
I skimmed some of the patches and this looks great!
Yay!
Note that
JSOp::FunCall
no longer exists (bug 1760989), but I don't think that really affects these changes.
Yes, no changes were needed. All patches even still apply cleanly on central!
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Updated•2 months ago
|
Comment 34•1 month ago
|
||
Pushed by andre.bargull@gmail.com: https://hg.mozilla.org/integration/autoland/rev/a01bac0bae2b Part 1: Add InlinableNativeIRGenerator. r=jandem https://hg.mozilla.org/integration/autoland/rev/b892b6743a0e Part 2: Use CallFlags member. r=jandem https://hg.mozilla.org/integration/autoland/rev/db6d5a84f079 Part 3: Delegate isFirstStub and script to CallIRGenerator. r=jandem https://hg.mozilla.org/integration/autoland/rev/1e71f02ed6fd Part 4: Replace JSOp member. r=jandem https://hg.mozilla.org/integration/autoland/rev/3f22f4b57103 Part 5: Pass CallFlags to tryAttachInlinableNative(). r=jandem https://hg.mozilla.org/integration/autoland/rev/935b2b91d4bb Part 6: Store callee as HandleFunction. r=jandem https://hg.mozilla.org/integration/autoland/rev/41437de94eb4 Part 7: Remove JSFunction argument from emitNativeCalleeGuard. r=jandem https://hg.mozilla.org/integration/autoland/rev/df647f3952c2 Part 8: Remove HandleFunction argument from tryAttach methods. r=jandem https://hg.mozilla.org/integration/autoland/rev/6d94cb999968 Part 9: Add wrapper around setInputOperandId. r=jandem https://hg.mozilla.org/integration/autoland/rev/3d5b7428d113 Part 10: Inline native functions through Function.prototype.call. r=jandem
Comment 35•1 month ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a01bac0bae2b
https://hg.mozilla.org/mozilla-central/rev/b892b6743a0e
https://hg.mozilla.org/mozilla-central/rev/db6d5a84f079
https://hg.mozilla.org/mozilla-central/rev/1e71f02ed6fd
https://hg.mozilla.org/mozilla-central/rev/3f22f4b57103
https://hg.mozilla.org/mozilla-central/rev/935b2b91d4bb
https://hg.mozilla.org/mozilla-central/rev/41437de94eb4
https://hg.mozilla.org/mozilla-central/rev/df647f3952c2
https://hg.mozilla.org/mozilla-central/rev/6d94cb999968
https://hg.mozilla.org/mozilla-central/rev/3d5b7428d113
Updated•1 month ago
|
Description
•