835277 - BaselineCompiler: Compile eval frames

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

We want to compile (at least) the same scripts JM compiles, so that means we have to support eval frames.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: WIP

Passes jit-tests on x86.

The debugger tests use eval extensively, so this patch also fixes some small debugger issues.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Part 1: Convert CallObject::createForStrictEval to use AbstractFramePtr

Patch for inbound, pretty straight-forward.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Part 2: Fix hasBreakpointsAt

The baseline compiler uses script->hasBreakpointsAt(pc) when toggling breakpoints/traps in the script. We can call this function (via destroy -> dec -> recompile) when enabledCount == 0 but the BreakpointSite is not yet destroyed.

Not really related to eval, but this fixes 2 failures introduced by other patches in this bug, so I'm just attaching it here.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Part 3: Call IonFrameIterator::script in GetTopIonJSScript

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Part 4: Compile eval scripts

This patch depends on the other patches in this bug (I will land these on mozilla-inbound first).

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Part 3: Call IonFrameIterator::script in GetTopIonJSScript

Inline IonFrameIterator::calleeToken() as well.

Comment 7

[Kannan Vijayan [:djvj]]

Comment on attachment 707588 [details] [diff] [review]
Part 4: Compile eval scripts

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

Nice and clean and very understandable code :)

::: js/src/ion/BaselineJIT.cpp
@@ +160,2 @@
>          // Single transition point from Interpreter to Ion.
> +        enter(jitcode, maxArgc, maxArgv, fp, calleeToken, evalScopeChain, &result);

Nit: fix comment above.  "from Interpreter to Baseline" instead of "from Interpreter to Ion"

::: js/src/ion/arm/Trampoline-arm.cpp
@@ +168,5 @@
>      masm.finishDataTransfer();
>  
> +    if (type == EnterJitBaseline) {
> +        JS_ASSERT(R1.scratchReg() != r0);
> +        masm.ma_mov(slot_evalScopeChain, R1.scratchReg());

Can't use the slot_evalScopeChain here, since it's based off of |sp|, and |sp| is modified by the data transfer that occurs before this code (they're effectively hidden pushes).

Since all the relevant registers are saved by the end of the first data transfer, you may want to take a register at that point, save sp, and use use that for the address that refers to the eval scope chain.

Small side note: the naming of the fields in register fields in EnterJITStack is in the wrong order.  |lr| should be the first field and |r0| should be the last field before |token| and |vp|.  This is not your bug though.

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Part 3: Call IonFrameIterator::script in GetTopIonJSScript

Inlining script() is annoying because it requires including IonFrames-inl.h in more files on some platforms. I don't think it's necessary so this just calls script().

Comment 9

[Jan de Mooij [:jandem]]

Part 1-3:

https://hg.mozilla.org/integration/mozilla-inbound/rev/e2d33e01fac4
https://hg.mozilla.org/integration/mozilla-inbound/rev/ccd9b78a80de
https://hg.mozilla.org/integration/mozilla-inbound/rev/585183fb7a13

Comment 10

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/projects/ionmonkey/rev/6a7f35be6722

(In reply to Kannan Vijayan [:djvj] from comment #7)
> Can't use the slot_evalScopeChain here, since it's based off of |sp|, and
> |sp| is modified by the data transfer that occurs before this code (they're
> effectively hidden pushes).

Oops, good catch! Fixed by using r11.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e2d33e01fac4
https://hg.mozilla.org/mozilla-central/rev/ccd9b78a80de
https://hg.mozilla.org/mozilla-central/rev/585183fb7a13