Last Comment Bug 724751 - IonMonkey: Change shell behavior so --ion -n is default
: IonMonkey: Change shell behavior so --ion -n is default
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: David Anderson [:dvander]
:
Mentors:
: 729804 (view as bug list)
Depends on: 747902
Blocks: IonMonkey LandIon
  Show dependency treegraph
 
Reported: 2012-02-06 19:18 PST by David Anderson [:dvander]
Modified: 2012-04-23 07:13 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (14.03 KB, patch)
2012-04-19 13:24 PDT, David Anderson [:dvander]
jdemooij: review+
Details | Diff | Review
follow-up (4.36 KB, patch)
2012-04-19 14:12 PDT, David Anderson [:dvander]
jdemooij: review+
Details | Diff | Review

Description David Anderson [:dvander] 2012-02-06 19:18:39 PST
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.
Comment 1 David Anderson [:dvander] 2012-02-27 18:28:37 PST
*** Bug 729804 has been marked as a duplicate of this bug. ***
Comment 2 David Anderson [:dvander] 2012-04-19 13:24:22 PDT
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.
Comment 3 Jan de Mooij [:jandem] 2012-04-19 13:43:08 PDT
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?
Comment 4 Gary Kwong [:gkw] [:nth10sd] 2012-04-19 14:10:33 PDT
> 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?
Comment 5 David Anderson [:dvander] 2012-04-19 14:12:12 PDT
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.
Comment 6 David Anderson [:dvander] 2012-04-19 14:15:33 PDT
(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 7 Jan de Mooij [:jandem] 2012-04-19 14:44:12 PDT
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)
Comment 8 David Anderson [:dvander] 2012-04-19 15:04:00 PDT
http://hg.mozilla.org/projects/ionmonkey/rev/300ac3d58291

Note You need to log in before you can comment on or make changes to this bug.