IonMonkey: Compile JSOP_LAMBDA and JSOP_DEFLOCALFUN

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Updated

5 years ago
Summary: IonMonkey: Compile JSOP_LAMBDA → IonMonkey: Compile JSOP_LAMBDA and JSOP_DEFLOCALFUN
(Assignee)

Comment 1

5 years ago
Created attachment 596017 [details] [diff] [review]
Patch

JSOP_LAMBDA is one of the more complicated deffun*/lambda* ops because we can do some optimizations to avoid cloning the function object in some cases, I hope the other ops will be a bit simpler.
Attachment #596017 - Flags: review?(dvander)
Comment on attachment 596017 [details] [diff] [review]
Patch

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

::: js/src/ion/IonBuilder.cpp
@@ +3515,5 @@
> +IonBuilder::jsop_lambda(JSFunction *fun)
> +{
> +    JS_ASSERT(script->analysis()->usesScopeChain());
> +    current->pushSlot(info().scopeChainSlot());
> +    MDefinition *scopeChain = current->pop();

It should be okay to use getSlot here, if you want to.

::: js/src/jsanalyze.cpp
@@ +344,5 @@
>  
> +          case JSOP_DEFLOCALFUN:
> +          case JSOP_LAMBDA:
> +            usesScopeChain_ = true;
> +            isInlineable = false;

Why didn't JM need this to set usesScopeChain?
Attachment #596017 - Flags: review?(dvander) → review+
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/projects/ionmonkey/rev/73866db4e189

(In reply to David Anderson [:dvander] from comment #2)
> 
> It should be okay to use getSlot here, if you want to.

Done.

> 
> Why didn't JM need this to set usesScopeChain?

JM's lambda stubs call StackFrame::scopeChain(), which sets the scope chain to the callee's environment if HAS_SCOPECHAIN is not set. I discussed this with bhackett on IRC, there are two ways to determine the parent of the new function in IM:

1) Use the scope chain slot (and set usesScopChain_ to true)
2) Use LFunctionEnvironment.

According to bhackett a good argument for 1) is that it will be forward compatible with block compilation.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Depends on: 797185
You need to log in before you can comment on or make changes to this bug.