Closed Bug 724751 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dvander, Assigned: dvander)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

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.
Attached patch patchSplinter 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.

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?
Attached patch follow-upSplinter Review
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)
(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+
http://hg.mozilla.org/projects/ionmonkey/rev/300ac3d58291
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Depends on: 747902
You need to log in before you can comment on or make changes to this bug.