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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

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.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Patch

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(-)

Comment 2

[Nicolas B. Pierron [:nbp]]

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

[Jan de Mooij [:jandem]]

(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 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 752305 [details] [diff] [review]
Patch

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

It begins!

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Patch v2

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.

Comment 6

[Kannan Vijayan [:djvj]]

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?

Comment 7

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e1bca8b56470

(Try: https://tbpl.mozilla.org/?tree=Try&rev=4bab52e636eb)


(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 :)

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/e1bca8b56470