Note: There are a few cases of duplicates in user autocompletion which are being worked on.

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

RESOLVED FIXED in mozilla24

Status

()

Core
JavaScript Engine
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

unspecified
mozilla24
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 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.
(Assignee)

Updated

4 years ago
Blocks: 873155
(Assignee)

Comment 1

4 years ago
Created attachment 752305 [details] [diff] [review]
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(-)
Assignee: general → jdemooij
Status: NEW → ASSIGNED
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?
(Assignee)

Comment 3

4 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]
Patch

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

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

Comment 5

4 years ago
Created attachment 752649 [details] [diff] [review]
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.
Attachment #752305 - Attachment is obsolete: true
Attachment #752305 - Flags: review?(kvijayan)
Attachment #752649 - Flags: superreview+
Attachment #752649 - Flags: review?(kvijayan)
(Assignee)

Updated

4 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+
(Assignee)

Comment 7

4 years ago
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 :)
https://hg.mozilla.org/mozilla-central/rev/e1bca8b56470
Status: ASSIGNED → RESOLVED
Last Resolved: 4 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.