IonMonkey: Require baseline compilation, remove bailout-to-interpreter code

RESOLVED FIXED in mozilla24



6 years ago
6 years ago


(Reporter: jandem, Assigned: jandem)


Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)



6 years ago
Ion can either bailout to the interpreter or to baseline code. The former code is unused with the default browser/shell flags I think, so we can remove it if we make it impossible to run Ion-without-Baseline.

Making this depend on bug 857845 for now so that it won't get lost.


6 years ago
Blocks: 873155

Comment 1

6 years ago
Posted patch Patch (obsolete) — Splinter Review
The OSR-from-the-interpreter and bailout-to-interpreter code is unused now with Baseline and Ion enabled and it's complicating stack refactoring work.

The attached patch removes it and disables Ion when Baseline is disabled.

13 files changed, 39 insertions(+), 920 deletions(-)
Assignee: general → jdemooij
Attachment #752305 - Flags: superreview?(dvander)
Attachment #752305 - Flags: review?(kvijayan)
Many parts of our test suite assume that we are able to eagerly compile with IonMonkey.  How would that work?

Would we compile both the baseline and Ion, and enter Ion almost straight from the interpreter or fuzzers should pay careful attention that we are not adding more bugs into IonMonkey?

Comment 3

6 years ago
(In reply to Nicolas B. Pierron [:nbp] from comment #2)
> Many parts of our test suite assume that we are able to eagerly compile with
> IonMonkey.  How would that work?

We can enter Ion the first time we see a LOOPENTRY op. What we can also do is always compile with Baseline first in ion::CanEnter if --ion-eager is used. That way we can still enter Ion immediately (and use the baseline code for when we bailout). Should be only a few lines of code and I agree it's probably good for fuzzing/tests.
Comment on attachment 752305 [details] [diff] [review]

Review of attachment 752305 [details] [diff] [review]:

It begins!
Attachment #752305 - Flags: superreview?(dvander) → superreview+

Comment 5

6 years ago
Posted patch Patch v2Splinter Review
Forces a Baseline compile with --ion-eager, so that we can directly enter Ion. This caught a (pre-existing) bug, patch for that coming up too.
Attachment #752305 - Attachment is obsolete: true
Attachment #752305 - Flags: review?(kvijayan)
Attachment #752649 - Flags: superreview+
Attachment #752649 - Flags: review?(kvijayan)


6 years ago
Depends on: 874825
Comment on attachment 752649 [details] [diff] [review]
Patch v2

Review of attachment 752649 [details] [diff] [review]:

::: js/src/ion/Ion.cpp
@@ -1988,5 @@
>          enter(jitcode, maxArgc, maxArgv, fp, calleeToken, /* scopeChain = */ NULL, 0,
>                result.address());
>      }
> -    if (result.isMagic() && result.whyMagic() == JS_ION_BAILOUT) {

Should we do a JS_ASSERT(!result.isMagic()) here now?
Attachment #752649 - Flags: review?(kvijayan) → review+

Comment 7

6 years ago


(In reply to Kannan Vijayan [:djvj] from comment #6)
> Should we do a JS_ASSERT(!result.isMagic()) here now?

There's a JS_ASSERT_IF(result.isMagic(), result.isMagic(JS_ION_ERROR)) at the end of the function :)
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Depends on: 880228
You need to log in before you can comment on or make changes to this bug.