1738413 - Inline inlinable natives through JSOp::FunCall

Status: RESOLVED FIXED

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

Description

[André Bargull [:anba] [back from break; working through mail backlog]]

Supporting inlining through inlinable natives when called from JSOp::FunCall. This helps for patterns like Object.prototype.toString.call(thing).

Comment 1

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 1: Add InlinableNativeCallInfo. r=jandem!

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.

Comment 2

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 2: Convert Array functions to InlinableNativeCallInfo. r=jandem!

Replaces the accesses to argc_, args_, callee_, newTarget_, and the
CallFlags to use InlinableNativeCallInfo.

Depends on D129903

Comment 3

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 3: Convert DataView natives to InlinableNativeCallInfo. r=jandem!

Replaces the accesses to argc_, args_, callee_, newTarget_, and the
CallFlags.

Depends on D129904

Comment 4

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 4: Convert intrinsics to InlinableNativeCallInfo. r=jandem!

Replaces the accesses to argc_, args_, callee_, newTarget_, and the
CallFlags.

Depends on D129905

Comment 5

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 5: Convert RegExp intrinsics to InlinableNativeCallInfo. r=jandem!

Replaces the accesses to argc_, args_, callee_, newTarget_, and the
CallFlags.

Depends on D129906

Comment 6

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 6: Convert String natives to InlinableNativeCallInfo. r=jandem!

Replaces the accesses to argc_, args_, callee_, newTarget_, and the
CallFlags.

Depends on D129907

Comment 7

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 7: Convert Math natives to InlinableNativeCallInfo. r=jandem!

Replaces the accesses to argc_, args_, callee_, newTarget_, and the
CallFlags.

Depends on D129908

Comment 8

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 8: Convert Map and Set natives to InlinableNativeCallInfo. r=jandem!

Replaces the accesses to argc_, args_, callee_, newTarget_, and the
CallFlags.

Depends on D129909

Comment 9

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 9: Convert Boolean, Number, and BigInt natives to InlinableNativeCallInfo. r=jandem!

Replaces the accesses to argc_, args_, callee_, newTarget_, and the
CallFlags.

Depends on D129910

Comment 10

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 10: Convert Object and Reflect natives to InlinableNativeCallInfo. r=jandem!

Replaces the accesses to argc_, args_, callee_, newTarget_, and the
CallFlags.

Depends on D129911

Comment 11

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 11: Convert TypedArray and ArrayBuffer intrinsics to InlinableNativeCallInfo. r=jandem!

Replaces the accesses to argc_, args_, callee_, newTarget_, and the
CallFlags.

Depends on D129912

Comment 12

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 12: Convert Atomics natives to InlinableNativeCallInfo. r=jandem!

Replaces the accesses to argc_, args_, callee_, newTarget_, and the
CallFlags.

Depends on D129913

Comment 13

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 13: Convert testing functions to InlinableNativeCallInfo. r=jandem!

Replaces the accesses to argc_, args_, callee_, newTarget_, and the
CallFlags.

Depends on D129914

Comment 14

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 14: Convert constructors to InlinableNativeCallInfo. r=jandem!

Replaces the accesses to argc_, args_, callee_, newTarget_, and the
CallFlags.

Depends on D129915

Comment 15

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 15: Convert spread calls to InlinableNativeCallInfo. r=jandem!

Replaces the accesses to argc_, args_, callee_, newTarget_, and the
CallFlags.

Depends on D129916

Comment 16

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 16: Use CallFlags from InlinableNativeCallInfo. r=jandem!

Depends on D129917

Comment 17

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: WIP: Bug 1738413 - Part 17: Inline native functions through Function.prototype.call. r=jandem!

To support inlining natives through JSOp::FunCall, we now just have to fill in
InlinableNativeCallInfo with the original this-value removed.

Depends on D129918

Comment 18

[André Bargull [:anba] [back from break; working through mail backlog]]

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

[Jan de Mooij [:jandem]]

(In reply to André Bargull [:anba] from comment #18)

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.

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?

Comment 20

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: Bug 1738413 - Part 1: Add InlinableNativeIRGenerator. r=jandem!

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.

Comment 21

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: Bug 1738413 - Part 2: Use CallFlags member. r=jandem!

Use the existing flags_ member field instead of recomputing CallFlags.
This also allows to remove the pc_ member.

Depends on D132125

Comment 22

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: Bug 1738413 - Part 3: Delegate isFirstStub and script to CallIRGenerator. r=jandem!

Change isFirstStub_ and script_ to methods which delegate to CallIRGenerator.

Depends on D132126

Comment 23

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: Bug 1738413 - Part 4: Replace JSOp member. r=jandem!

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

Comment 24

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: Bug 1738413 - Part 5: Pass CallFlags to tryAttachInlinableNative(). r=jandem!

Pass CallFlags to fix the "TODO" note.

Depends on D132128

Comment 25

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: Bug 1738413 - Part 6: Store callee as HandleFunction. r=jandem!

The callee is guaranteed to be a JSFunction, so we can use HandleFunction
instead of HandleValue.

Depends on D132129

Comment 26

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: Bug 1738413 - Part 7: Remove JSFunction argument from emitNativeCalleeGuard. r=jandem!

emitNativeCalleeGuard() can just read the callee from the member field instead
of receiving it as a parameter.

Depends on D132130

Comment 27

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: Bug 1738413 - Part 8: Remove HandleFunction argument from tryAttach methods. r=jandem!

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

Comment 28

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: Bug 1738413 - Part 9: Add wrapper around setInputOperandId. r=jandem!

This is in preparation for the next part, where we don't want to call
setInputOperandId() for JSOp::FunCall calls.

Depends on D132132

Comment 29

[André Bargull [:anba] [back from break; working through mail backlog]]

Attachment: Bug 1738413 - Part 10: Inline native functions through Function.prototype.call. r=jandem!

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:

1. writer.setInputOperandId(0) mustn't be called a second time.
2. 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

Comment 30

[André Bargull [:anba] [back from break; working through mail backlog]]

(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 move tryAttachMathAbs etc to a new class that contains the fields in InlinableNativeCallInfo + the writer and some other things?

I've uploaded another patch set with the proposed design.

Comment 31

[André Bargull [:anba] [back from break; working through mail backlog]]

(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 move tryAttachMathAbs etc to a new class that contains the fields in InlinableNativeCallInfo + the writer and some other things?

I've uploaded another patch set with the proposed design.

NI in case you've missed the updates.

Comment 32

[Jan de Mooij [:jandem]]

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

Comment 33

[André Bargull [:anba] [back from break; working through mail backlog]]

(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!

Comment 34

[Pulsebot]

Pushed by [e-mail]:
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

[Norisz Fay [:noriszfay]]

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