1411954 - Clean up cloning of native functions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P3)

Description

[Jan de Mooij [:jandem]]

Attachment: 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).

Comment 1

[Jeff Walden [:Waldo]]

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.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e958dfd8be0d
Move cloning of native functions into separate functions. r=jwalden

Comment 3

[Jan de Mooij [:jandem]]

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

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/e958dfd8be0d