Last Comment Bug 725674 - IonMonkey: Compile JSOP_LAMBDA and JSOP_DEFLOCALFUN
: IonMonkey: Compile JSOP_LAMBDA and JSOP_DEFLOCALFUN
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Jan de Mooij [:jandem]
:
Mentors:
Depends on: 797185
Blocks: 684381
  Show dependency treegraph
 
Reported: 2012-02-09 08:34 PST by Jan de Mooij [:jandem]
Modified: 2012-10-02 16:00 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (38.69 KB, patch)
2012-02-10 04:39 PST, Jan de Mooij [:jandem]
dvander: review+
Details | Diff | Splinter Review

Description Jan de Mooij [:jandem] 2012-02-09 08:34:42 PST

    
Comment 1 Jan de Mooij [:jandem] 2012-02-10 04:39:17 PST
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.
Comment 2 David Anderson [:dvander] 2012-02-10 14:52:32 PST
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] 2012-02-11 05:54:51 PST
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.

Note You need to log in before you can comment on or make changes to this bug.