IonMonkey: Change shell behavior so --ion -n is default

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dvander, Assigned: dvander)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
Patch soon - this will change the shell such that --ion -n is default (though existing behavior is preserved). In addition there is new behavior:
  --ti=[on/off], and
  --ion=[on/off].

This patch will also disable Ion if type inference is not enabled, because the DummyOracle is not usable.
(Assignee)

Updated

5 years ago
Duplicate of this bug: 729804
(Assignee)

Updated

5 years ago
Blocks: 745386
(Assignee)

Comment 2

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

The first part of this patch changes shell flags:
  --ion is default, and removed. --no-ion disables.
  -n is default, and removed. --no-ti disables.
  -m is default, and removed. --no-jm disables.

The second part changes jit_tests and make check:
  --ion-tbpl is now --tbpl and tests all salient JIT combinations.

The last part changes how JM decides to compile, to give IonMonkey a sure chance at compiling first.
Attachment #616725 - Flags: review?(jdemooij)
Comment on attachment 616725 [details] [diff] [review]
patch

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

Looks good, enabling Ion by default may be confusing at first but it seems like it's the right thing to do.

::: js/src/Makefile.in
@@ +686,2 @@
>  
>  check:: check-jit-test check-ion-test

You can remove check-ion-test here too.

::: js/src/shell/js.cpp
@@ +4662,1 @@
>          JS_ToggleOptions(cx, JSOPTION_METHODJIT);

Do we have to remove this line, like -n/--no-ti?
Attachment #616725 - Flags: review?(jdemooij) → review+
> The first part of this patch changes shell flags:
>   --ion is default, and removed. --no-ion disables.
>   -n is default, and removed. --no-ti disables.
>   -m is default, and removed. --no-jm disables.

-a will still have to be passed in separately, right?
(Assignee)

Comment 5

5 years ago
Created attachment 616742 [details] [diff] [review]
follow-up

Thanks, actually --no-ti needed a call to JS_ToggleOptions. I made --no-ion work the same way too now.

Here's a follow-up patch to make IonEnabled return false if TI is off. This also disables using the dummy oracle.
Attachment #616742 - Flags: review?(jdemooij)
(Assignee)

Comment 6

5 years ago
(In reply to Gary Kwong [:gkw, :nth10sd] from comment #4)
> -a will still have to be passed in separately, right?

Yeah. The other options continue to work as normal.
Comment on attachment 616742 [details] [diff] [review]
follow-up

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

Can we remove the dummy oracle? (does not have to happen in this bug)
Attachment #616742 - Flags: review?(jdemooij) → review+
(Assignee)

Comment 8

5 years ago
http://hg.mozilla.org/projects/ionmonkey/rev/300ac3d58291
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Depends on: 747902
You need to log in before you can comment on or make changes to this bug.