791759 - Fix jit-tests default jitflags

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

jit-test.py with no arguments runs each test once with -m and once with -m -n. This no longer makes sense though, with the IonMonkey merge JM, TI and Ion are now enabled by default so we run each test twice with the same configuration.

We should clean-up the jitflags stuff and I propose by default we run each test with (1) --no-jm and (2) --no-ion.

TBPL can test the other flags like --no-ti and --ion-eager (and note that TBPL already uses the right flags).

Comment 1

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #0)
> 
> We should clean-up the jitflags stuff and I propose by default we run each
> test with (1) --no-jm and (2) --no-ion.

Or rather (1) --no-jm and (2) no flags.

Comment 2

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

Do we even need --no-jm? But yeah either way, this sounds good to me.

Comment 3

[Jan de Mooij [:jandem]]

What if we remove the --jitflags argument entirely and by default run the shell with no arguments. If you want to run with other arguments you can either

(1) Use --args: jit_test.py --args="--no-ti -a -d"
(2) Use --ion to run with --no-jm/--ion-eager
(3) Use --tbpl to run with all tinderbox flags

dmandelin, terrence, does this sound reasonable?

Comment 4

[David Mandelin [:dmandelin]]

(In reply to Jan de Mooij [:jandem] from comment #3)
> What if we remove the --jitflags argument entirely and by default run the
> shell with no arguments. If you want to run with other arguments you can
> either
> 
> (1) Use --args: jit_test.py --args="--no-ti -a -d"
> (2) Use --ion to run with --no-jm/--ion-eager
> (3) Use --tbpl to run with all tinderbox flags
> 
> dmandelin, terrence, does this sound reasonable?

I would say:

 - jit_test.py by default should do the thing that is most useful to devs in day-to-day work
 - it needs whatever other options devs end up finding useful
 - Tinderbox needs to run whatever is appropriate for catching bugs in continuous integration and shipping a high-quality product

But it's up to you, the JIT people, to decide what that actually means.

(Feedback on option names: --args, --tbpl are nice and clear; --ion is ambiguous, --ion-eager or --ion-only would be better)

Comment 5

[Nicolas B. Pierron [:nbp] {away}]

(In reply to David Anderson [:dvander] from comment #2)
> Do we even need --no-jm? But yeah either way, this sounds good to me.

I found --no-jm useful for cases where we need to pre-heat the script before compiling, it act as some-kind of ion eager with higher threshold.

Comment 6

[Till Schneidereit [:till]]

Attachment: Remove default flags

I just stumbled across this. As we currently run each test twice with effectively the same configuration, I think we should fix this in the short term by just removing the default flags.

I also added some documentation about flags.

Comment 7

[David Mandelin [:dmandelin]]

Comment on attachment 683222 [details] [diff] [review]
Remove default flags

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

Makes sense to me.

Comment 8

[Till Schneidereit [:till]]

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

Comment 9

[Ed Morley [:emorley]]

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