Clean up cloning of native functions

RESOLVED FIXED in Firefox 58

Status

()

Core
JavaScript Engine
P3
normal
RESOLVED FIXED
27 days ago
21 days ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

unspecified
mozilla58
Points:
---

Firefox Tracking Flags

(firefox57 wontfix, firefox58 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

27 days ago
Created attachment 8922302 [details] [diff] [review]
Patch

After bug 1405766, the only native functions we clone are self-hosting intrinsics and AsmJS module function lambdas. We can add separate functions for these cases and then CloneFunctionAndScript, CloneFunctionReuseScript, CanReuseScriptForClone are only used for interpreted functions and things are a lot less confusing.

A nice side-effect from this is that we no longer allocate an extended function for self-hosting intrinsics (because the name that's stored in an extended slot is only used for delazification of interpreted functions).
Attachment #8922302 - Flags: review?(jwalden+bmo)
(Assignee)

Updated

27 days ago
status-firefox57: --- → wontfix
Priority: -- → P3
Comment on attachment 8922302 [details] [diff] [review]
Patch

Review of attachment 8922302 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsfun.cpp
@@ +2276,5 @@
> +                                         /* proto = */ nullptr);
> +    if (!clone)
> +        return nullptr;
> +
> +    clone->initNative(fun->native(), fun->jitInfo());

I'd prefer if we asserted the identity of the native/jitinfo here as InstantiateAsmJS and (?) nullptr, then used constants directly here.  Cut off dataflow dependencies at the earliest possible point for bespoke functions, and all.
Attachment #8922302 - Flags: review?(jwalden+bmo) → review+

Comment 2

22 days ago
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e958dfd8be0d
Move cloning of native functions into separate functions. r=jwalden
(Assignee)

Comment 3

22 days ago
(In reply to Jeff Walden [:Waldo] (I'm baaaaaaack...) from comment #1)
> I'd prefer if we asserted the identity of the native/jitinfo here as
> InstantiateAsmJS and (?) nullptr, then used constants directly here.  Cut
> off dataflow dependencies at the earliest possible point for bespoke
> functions, and all.

Great idea.
https://hg.mozilla.org/mozilla-central/rev/e958dfd8be0d
Status: ASSIGNED → RESOLVED
Last Resolved: 21 days ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.