Closed Bug 743394 Opened 8 years ago Closed 7 years ago

IonMonkey: Handle JSOP_EVAL

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: sstangl, Assigned: bhackett)

References

(Blocks 1 open bug)

Details

(Whiteboard: [ion:t])

Attachments

(1 file)

The main function of SS-1.0's date-format-tofte cannot compile due to JSOP_EVAL in a loop.
This probably depends on scope work because eval -> heavyweight -> needs call object.
date-format-xparb also.
This seems to be the main thing slowing down Baseline+Ion vs. JM+Ion right now on SS.  We don't Ion compile the function where tofte spends most of its time, so it always runs in Baseline, which is considerably slower than JM+TI.

The new stuff needed seems to be (a) call a stub for JSOP_EVAL, and (b) create an args object for the frame, which will be required.  It'd be nice if (b) wasn't actually necessary, and the arguments object was just created if the eval() actually tried to access it.
Blocks: 842167
Depends on: 842522
Attached patch patchSplinter Review
This (roughly) applies on top of bug 842522.
Assignee: general → bhackett1024
Attachment #715722 - Flags: review?(jdemooij)
Comment on attachment 715722 [details] [diff] [review]
patch

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

Looks good. r=me with comments below addressed.

::: js/src/vm/Stack-inl.h
@@ +930,5 @@
> +        if (prev->beginsIonActivation()) {
> +            /*
> +             * Walk the stack to find the previous frame's scope chain. Don't
> +             * do this in other cases as this walk does not work when the eval
> +             * comes from an eval-in-frame.

Why does it not work? Eval-in-frame links fp->prev_ to the frame in which we are evaluating the expression, and StackIter just follows prev_. To avoid special handling for eval-in-frame with baseline frames, we should always use StackIter/ScriptFrameIter.

@@ +934,5 @@
> +             * comes from an eval-in-frame.
> +             */
> +            StackIter iter(cx);
> +            while (iter.script() != script())
> +                ++iter;

Use ScriptFrameIter to filter out natives etc (will probably fix eval-in-frame failures too) and if the same script is eval'ed recursively it's safer to do:

/* Ion does not compile eval frames. */
while (iter.isIon() || iter.abstractFramePtr() != *this)
    ++iter;
Attachment #715722 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/0ded3af9b2d7
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Depends on: 847412
Depends on: 922118
You need to log in before you can comment on or make changes to this bug.