Closed
Bug 735400
Opened 13 years ago
Closed 13 years ago
IonMonkey: Optimize JSOP_FUNCALL
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: dvander, Assigned: sstangl)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
Needed for deltablue and earley-boyer, I think. Ideally we want:
f.call(x, ...)
To become:
f(...) ; where this = x
We should be able to inline through in the best case (I think JM+TI does this), and for other cases fall back to a transformation to MCall.
Assignee | ||
Updated•13 years ago
|
Assignee: general → sstangl
Assignee | ||
Comment 1•13 years ago
|
||
We already support JSOP_FUNCALL as described. The hard case for JSOP_THIS isn't handled, but that is a separate bug.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WORKSFORME
Assignee | ||
Comment 2•13 years ago
|
||
I've been tricked. Dvander pointed out that we don't really handle JSOP_FUNCALL at all -- we go through a native. Reopening.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Assignee | ||
Comment 3•13 years ago
|
||
This implements JSOP_FUNCALL optimizations without inlining. The normal call mechanism is reused by shifting the slots down by one, overwriting the native, and unwrapping the MPassArg(JSFunction *).
Unfortunately, logic to inline functions queries TI for safety/argument information, and with the transformation of f.call(...) -> f(...), the types returned by TI no longer correspond to the MIR being built.
The obvious suggestion is to collect TI information before shimmying the slots down, but native inlining needs access to arbitrary many TypeSets. Another option is to have poppedTypes() check whether pc == JSOP_FUNCALL in certain situations, and if so, actually check one slot higher. Doesn't sound pleasant.
Assignee | ||
Comment 4•13 years ago
|
||
Comment on attachment 609518 [details] [diff] [review]
JSOP_FUNCALL without inlining.
JM doesn't inline, so it's probably not awful for us to also not inline.
v8-v6 deltablue goes ~500ms -> ~480ms on local x86; all other tests unaffected.
Attachment #609518 -
Flags: review?(dvander)
Reporter | ||
Comment 5•13 years ago
|
||
Comment on attachment 609518 [details] [diff] [review]
JSOP_FUNCALL without inlining.
Review of attachment 609518 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/ion/IonBuilder.cpp
@@ +125,5 @@
> return state;
> }
>
> +JSObject *
> +IonBuilder::getSingleObject(types::TypeSet *types)
nit: This looks like it could be a static, instead of a member function (maybe not even declared in IonBuilder.h)
@@ +129,5 @@
> +IonBuilder::getSingleObject(types::TypeSet *types)
> +{
> + if (!types || types->unknownObject() || types->getObjectCount() != 1)
> + return NULL;
> + return types->getSingleObject(0);
I think this can just be:
if (!types)
return NULL;
return types->getSingleton();
@@ +2566,5 @@
> + if (!native || !native->isNative() || native->native() != &js_fun_call)
> + return makeCall(native, argc, false);
> +
> + // Extract call target.
> + JSObject *funobj = getSingleObject(oracle->getCallArg(script, argc, 0, pc));
Are we guaranteed a non-null funobj here?
::: js/src/ion/MIRGraph.h
@@ +142,5 @@
> // not be used under most circumstances.
> void rewriteSlot(uint32 slot, MDefinition *ins);
>
> + // Rewrites a slot based on its current depth (same as argument to peek()).
> + void rewriteDepth(int32 depth, MDefinition *ins);
Maybe "rewriteAtDepth" would be a better name.
Attachment #609518 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 6•13 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•