725674 - IonMonkey: Compile JSOP_LAMBDA and JSOP_DEFLOCALFUN

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Jan de Mooij [:jandem]]

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

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

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?

Comment 3

[Jan de Mooij [:jandem]]

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.