Closed
Bug 1117146
Opened 10 years ago
Closed 10 years ago
jit-test should run asm.js subdirectory with asm.js disabled.
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: nbp, Assigned: nbp)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
(deleted),
patch
|
bbouvier
:
review+
luke
:
feedback+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
nbp
:
review+
|
Details | Diff | Splinter Review |
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.
Comment 1•10 years ago
|
||
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)
Assignee | ||
Comment 2•10 years ago
|
||
(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)
Assignee | ||
Comment 3•10 years ago
|
||
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)
Assignee | ||
Comment 4•10 years ago
|
||
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 5•10 years ago
|
||
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 6•10 years ago
|
||
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+
Assignee | ||
Comment 7•10 years ago
|
||
Comment 8•10 years ago
|
||
Assignee: nobody → nicolas.b.pierron
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment 9•10 years ago
|
||
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)?
Assignee | ||
Comment 10•10 years ago
|
||
(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='…'.
Comment 11•10 years ago
|
||
Yes, that would be great, thank you!
Comment 12•10 years ago
|
||
Attachment #8553132 -
Flags: review?(nicolas.b.pierron)
Assignee | ||
Updated•10 years ago
|
Attachment #8553132 -
Flags: review?(nicolas.b.pierron) → review+
Comment 13•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•