Closed Bug 725674 Opened 12 years ago Closed 12 years ago

IonMonkey: Compile JSOP_LAMBDA and JSOP_DEFLOCALFUN

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

      No description provided.
Summary: IonMonkey: Compile JSOP_LAMBDA → IonMonkey: Compile JSOP_LAMBDA and JSOP_DEFLOCALFUN
Attached patch PatchSplinter Review
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+
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
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.