IonMonkey: Handle JSOP_EVAL

RESOLVED FIXED in mozilla22

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: sstangl, Assigned: bhackett)

Tracking

unspecified
mozilla22
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ion:t])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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.
(Reporter)

Comment 2

6 years ago
date-format-xparb also.
(Assignee)

Comment 3

5 years ago
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
(Assignee)

Updated

5 years ago
Depends on: 842522
(Assignee)

Comment 4

5 years ago
Created attachment 715722 [details] [diff] [review]
patch

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
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Depends on: 847412

Updated

5 years ago
Depends on: 922118
You need to log in before you can comment on or make changes to this bug.