Closed Bug 1117146 Opened 5 years ago Closed 5 years ago

jit-test should run asm.js subdirectory with asm.js disabled.

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: nbp, Assigned: nbp)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Currently asm.js tests are only ran with Odin enabled, we should run asm.js tests twice with Odin disabled as well.  This would prevent duplicating the test cases, and should catch differential behaviour between asmjs and other mode of executions.

Currently, jit-tests have a way to provide additional options for running the tests.  We should extend the per-file jit-tests comment to flag the directory to be run all the tests of this directory with additional flags.
Depends on: 1117148
It might take too slow if we have exotic combinations like --no-asmjs --no-baseline. Nicolas, do you have measurements of how much time it would take to run the entire tests subdirectory (skipping testSIMD at the moment) with

--no-asmjs --no-baseline
--no-asmjs --no-ion
--no-asmjs
Flags: needinfo?(nicolas.b.pierron)
(In reply to Benjamin Bouvier [:bbouvier] from comment #1)
> It might take too slow if we have exotic combinations like --no-asmjs
> --no-baseline. Nicolas, do you have measurements of how much time it would
> take to run the entire tests subdirectory (skipping testSIMD at the moment)
> with
> 
> --no-asmjs --no-baseline
> --no-asmjs --no-ion
> --no-asmjs

Using "jit-test/jit_test.py --tbpl --args=--no-asmjs ./js asm.js" takes 51s (--ion takes 14s) with a debug build, which does not sounds like a lot.
Flags: needinfo?(nicolas.b.pierron)
This patch add a |jit-test| asmjs-compat flag such that one test can be
executed with all the default variants, and with an additional one which is
used to disable asmjs.
Attachment #8544063 - Flags: review?(benj)
Attachment #8544063 - Flags: feedback?(luke)
Note that the overhead is 3.25s (debug) and 0.47s (optimized) when running these jit-tests, as opposed to --args=--no-asmjs.

I did not add the jit-test flag to test cases which are named "testBug" and "testTimeout", as they were likely to be testing something which is specific to asmjs.
Comment on attachment 8544063 [details] [diff] [review]
Add a |jit-test| flag to run an additional --no-asmjs variant for a test.

Cool!  We should have done this to start with.
Attachment #8544063 - Flags: feedback?(luke) → feedback+
Comment on attachment 8544063 [details] [diff] [review]
Add a |jit-test| flag to run an additional --no-asmjs variant for a test.

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

Cool, we'll add asm.js/testSIMD.js when the behavior is the same everywhere.

::: js/src/tests/lib/jittests.py
@@ +112,5 @@
>          self.allow_overrecursed = False # True means that hitting recursion the
>                                          # limits is not considered a failure.
>          self.valgrind = False  # True means run under valgrind
>          self.tz_pacific = False # True means force Pacific time for the test
> +        self.asmjs_compat = False # True means run with and without asm.js enabled.

I would prefer a more informative name, so that somebody who looks at a test file for the first time doesn't have to look up the meaning of the flag. How about "testAlsoNoAsm" or "testWithAndWithoutAsm" or something better?

@@ +144,5 @@
> +        if self.asmjs_compat:
> +            variants = variants + [['--no-asmjs']]
> +
> +        # For each list of jit flags, make a copy of the test.
> +        return [ self.copy_and_extend_jitflags(v) for v in variants ]

nice refactoring!

@@ +194,5 @@
>                          test.valgrind = options.valgrind
>                      elif name == 'tz-pacific':
>                          test.tz_pacific = True
> +                    elif name == 'asmjs-compat':
> +                        test.asmjs_compat = True

(if you change the option name up, don't forget to modify it here as well ;))
Attachment #8544063 - Flags: review?(benj) → review+
Blocks: 1119367
https://hg.mozilla.org/mozilla-central/rev/60d24d869b45
Assignee: nobody → nicolas.b.pierron
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Is there a convenient way to run a single test so that "-g" works (and doesn't give me an error about multiple tests match the command line)?
(In reply to Luke Wagner [:luke] from comment #9)
> Is there a convenient way to run a single test so that "-g" works (and
> doesn't give me an error about multiple tests match the command line)?

hum … no, as the test it-self request multiple variants, I guess we should disable this feature if we give the -g option, and let the developer give the correct --args='…'.
Yes, that would be great, thank you!
Attachment #8553132 - Flags: review?(nicolas.b.pierron) → review+
You need to log in before you can comment on or make changes to this bug.