Closed
Bug 1496297
Opened 7 years ago
Closed 7 years ago
Allow skip-if in jit-tests
Categories
(Core :: JavaScript Engine, enhancement, P2)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla64
Tracking | Status | |
---|---|---|
firefox64 | --- | fixed |
People
(Reporter: efaust, Assigned: efaust)
References
Details
Attachments
(23 files, 1 obsolete file)
5.95 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
40.03 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
18.49 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
23.27 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
32.59 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
63.33 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
46.43 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
2.89 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
6.16 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
14.99 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
1.49 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
853 bytes,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
5.82 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
838 bytes,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
2.02 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
8.34 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
5.28 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
2.12 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
5.87 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
13.93 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
1.39 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
4.41 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
1.63 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•7 years ago
|
||
Assignee: nobody → efaustbmo
Status: NEW → ASSIGNED
Attachment #9014251 -
Flags: feedback?(jorendorff)
Assignee | ||
Updated•7 years ago
|
Summary: allow skip-if in jit-tests → Allow skip-if in jit-tests
Comment 2•7 years ago
|
||
Comment on attachment 9014251 [details] [diff] [review]
[Part 1] Allow skip-if in jit-tests
Review of attachment 9014251 [details] [diff] [review]:
-----------------------------------------------------------------
This is basically exactly what I had.
Attachment #9014251 -
Flags: feedback?(jorendorff) → feedback+
Assignee | ||
Comment 3•7 years ago
|
||
Attachment #9014251 -
Attachment is obsolete: true
Attachment #9014528 -
Flags: review?(jorendorff)
Assignee | ||
Comment 4•7 years ago
|
||
Attachment #9014529 -
Flags: review?(jorendorff)
Assignee | ||
Comment 5•7 years ago
|
||
Attachment #9014531 -
Flags: review?(jorendorff)
Assignee | ||
Comment 6•7 years ago
|
||
Attachment #9014533 -
Flags: review?(jorendorff)
Assignee | ||
Comment 7•7 years ago
|
||
Attachment #9014534 -
Flags: review?(jorendorff)
Assignee | ||
Comment 8•7 years ago
|
||
Attachment #9014535 -
Flags: review?(jorendorff)
Assignee | ||
Comment 9•7 years ago
|
||
Attachment #9014536 -
Flags: review?(jorendorff)
Assignee | ||
Comment 10•7 years ago
|
||
Attachment #9014538 -
Flags: review?(jorendorff)
Assignee | ||
Comment 11•7 years ago
|
||
Attachment #9014540 -
Flags: review?(jorendorff)
Assignee | ||
Comment 12•7 years ago
|
||
Attachment #9014541 -
Flags: review?(jorendorff)
Assignee | ||
Comment 13•7 years ago
|
||
Attachment #9014542 -
Flags: review?(jorendorff)
Assignee | ||
Comment 14•7 years ago
|
||
Attachment #9014543 -
Flags: review?(jorendorff)
Assignee | ||
Comment 15•7 years ago
|
||
Attachment #9014544 -
Flags: review?(jorendorff)
Assignee | ||
Comment 16•7 years ago
|
||
Attachment #9014545 -
Flags: review?(jorendorff)
Assignee | ||
Comment 17•7 years ago
|
||
Attachment #9014546 -
Flags: review?(jorendorff)
Assignee | ||
Comment 18•7 years ago
|
||
Attachment #9014547 -
Flags: review?(jorendorff)
Assignee | ||
Comment 19•7 years ago
|
||
Attachment #9014548 -
Flags: review?(jorendorff)
Assignee | ||
Comment 20•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #9014549 -
Flags: review?(jorendorff)
Assignee | ||
Comment 21•7 years ago
|
||
Attachment #9014550 -
Flags: review?(jorendorff)
Assignee | ||
Comment 22•7 years ago
|
||
Attachment #9014551 -
Flags: review?(jorendorff)
Assignee | ||
Comment 23•7 years ago
|
||
Attachment #9014552 -
Flags: review?(jorendorff)
Assignee | ||
Comment 24•7 years ago
|
||
Attachment #9014553 -
Flags: review?(jorendorff)
Assignee | ||
Comment 25•7 years ago
|
||
Attachment #9014554 -
Flags: review?(jorendorff)
Assignee | ||
Comment 26•7 years ago
|
||
Comment 27•7 years ago
|
||
Comment on attachment 9014528 [details] [diff] [review]
Implement |jit-test| skip-if: cond
Review of attachment 9014528 [details] [diff] [review]:
-----------------------------------------------------------------
\o/
::: js/src/tests/lib/jittests.py
@@ +199,5 @@
> # For each list of jit flags, make a copy of the test.
> return [self.copy_and_extend_jitflags(v) for v in variants]
>
> COOKIE = '|jit-test|'
> + SKIPPED_RETURN = 59
SKIPPED_EXIT_STATUS please.
Attachment #9014528 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014529 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014531 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014533 -
Flags: review?(jorendorff) → review+
Comment 28•7 years ago
|
||
Comment on attachment 9014534 [details] [diff] [review]
Use |jit-test| skip-if in jit-test/tests/wasm
Review of attachment 9014534 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/jit-test/tests/wasm/timeout/1.js
@@ +2,3 @@
>
> // Don't include wasm.js in timeout tests: when wasm isn't supported, it will
> // quit(0) which will cause the test to fail.
Is this comment still correct? Same story in 2.js, debug-interrupt-1.js, etc.
Attachment #9014534 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014535 -
Flags: review?(jorendorff) → review+
Comment 29•7 years ago
|
||
Comment on attachment 9014536 [details] [diff] [review]
Use |jit-test| skip-if in jit-test/tests/debug
Review of attachment 9014536 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/jit-test/tests/debug/wasm-12.js
@@ +5,1 @@
>
Extreme style nit: remove the extra blank line here, please.
Attachment #9014536 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014538 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014540 -
Flags: review?(jorendorff) → review+
Comment 30•7 years ago
|
||
Comment on attachment 9014541 [details] [diff] [review]
Use |jit-test| skip-if in jit-test/tests/asm.js
Review of attachment 9014541 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/jit-test/tests/asm.js/testZOOB.js
@@ +1,1 @@
> +// |jit-test|; skip-if: !isAsmJSCompilationAvailable()
Nit: Stray semicolon here.
Attachment #9014541 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014542 -
Flags: review?(jorendorff) → review+
Comment 31•7 years ago
|
||
Comment on attachment 9014543 [details] [diff] [review]
Use |jit-test| skip-if in jit-test/tests/latin1
Review of attachment 9014543 [details] [diff] [review]:
-----------------------------------------------------------------
r=me with the confusion addressed one way or the other...
::: js/src/jit-test/tests/latin1/asm.js
@@ +7,1 @@
> quit();
Little confused about this one. Is this right? Should it throw if caching wasn't enabled?
Attachment #9014543 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014544 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014545 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014546 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014547 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014548 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014549 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014550 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014551 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014552 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014553 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Attachment #9014554 -
Flags: review?(jorendorff) → review+
Updated•7 years ago
|
Priority: -- → P2
Assignee | ||
Comment 32•7 years ago
|
||
(In reply to Jason Orendorff [:jorendorff] from comment #31)
> Comment on attachment 9014543 [details] [diff] [review]
> Use |jit-test| skip-if in jit-test/tests/latin1
>
> Review of attachment 9014543 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> r=me with the confusion addressed one way or the other...
>
> ::: js/src/jit-test/tests/latin1/asm.js
> @@ +7,1 @@
> > quit();
>
> Little confused about this one. Is this right? Should it throw if caching
> wasn't enabled?
Seems right. setCacheEnabled(true) sets the "I want to have this enabled" bool, but isCachingEnabled() checks whether there's a cachepath as well. Seems like we need both, and that skipping peacefully is fine.
Comment 33•7 years ago
|
||
Pushed by efaustbmo@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a7a0b6b2bbf
Implement skip-if in jit-tests. (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/012e94d3d96c
Use |jit-test| skip-if as appropriate in jit-test/tests/TypedObject/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/fdf9b230e270
Use |jit-test| skip-if as appropriate in jit-test/tests/basic/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/b97fa179df85
Use |jit-test| skip-if as appropriate in jit-test/tests/ion/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b29d20f0309
Use |jit-test| skip-if as appropriate to jit-test/tests/wasm/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/4157315ccd54
Use |jit-test| skip-if as appropriate in jit-test/tests/gc/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb0c1944852b
Use |jit-test| skip-if as appropriate in jit-test/tests/debug/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb27fccdb19c
Use |jit-test| skip-if as appropriate in jit-test/tests/baseline/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/5140037f3a17
Use |jit-test| skip-if as appropriate in jit-test/tests/auto-regress/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/b95948e9a126
Use |jit-test| skip-if as appropriate in jite-test/tests/asm.js/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b8297687615
Use |jit-test| skip-if as appropriate in jit-test/tests/regexp/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/9da32b3fe705
Use |jit-test| skip-if as appropriate in jit-test/tests/latin1/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/6112cec4211f
Use |jit-test| skip-if as appropriate in jit-test/tests/atomics/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/f6a61c3b3dbb
Use |jit-test| skip-if as appropriate in jit-test/tests/realms/ (r=jornedorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/085483ce64fe
Use |jit-test| skip-if as appropriate in jit-test/tests/saved-stacks/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca4e8e9425ae
Use |jit-test| skip-if as appropriate in jit-test/tests/sharedbuf/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/0fb70553886a
Use |jit-test| skip-if as appropriate in jit-test/tests/parser/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/178e8e65a1a3
Use |jit-test| skip-if as appropriate in jit-test/tests/profiler/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/4da5afa3f427
Use |jit-test| skip-if as appropriate in jit-test/tests/modules/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/222b9bd838ec
Use |jit-test| skip-if as appropriate in jit-test/tests/regexp_parse/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/de0ab3ea1796
Use |jit-test| skip-if as appropriate in jit-test/tests/xdr/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/179b81691182
Use |jit-test| skip-if as appropriate in jit-test/tests/heap-analysis/ (r=jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8b420c49253
Use |jit-test| skip-if as appropriate in jit-test/tests/ (r=jorendorff)
Comment 34•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2a7a0b6b2bbf
https://hg.mozilla.org/mozilla-central/rev/012e94d3d96c
https://hg.mozilla.org/mozilla-central/rev/fdf9b230e270
https://hg.mozilla.org/mozilla-central/rev/b97fa179df85
https://hg.mozilla.org/mozilla-central/rev/1b29d20f0309
https://hg.mozilla.org/mozilla-central/rev/4157315ccd54
https://hg.mozilla.org/mozilla-central/rev/cb0c1944852b
https://hg.mozilla.org/mozilla-central/rev/bb27fccdb19c
https://hg.mozilla.org/mozilla-central/rev/5140037f3a17
https://hg.mozilla.org/mozilla-central/rev/b95948e9a126
https://hg.mozilla.org/mozilla-central/rev/0b8297687615
https://hg.mozilla.org/mozilla-central/rev/9da32b3fe705
https://hg.mozilla.org/mozilla-central/rev/6112cec4211f
https://hg.mozilla.org/mozilla-central/rev/f6a61c3b3dbb
https://hg.mozilla.org/mozilla-central/rev/085483ce64fe
https://hg.mozilla.org/mozilla-central/rev/ca4e8e9425ae
https://hg.mozilla.org/mozilla-central/rev/0fb70553886a
https://hg.mozilla.org/mozilla-central/rev/178e8e65a1a3
https://hg.mozilla.org/mozilla-central/rev/4da5afa3f427
https://hg.mozilla.org/mozilla-central/rev/222b9bd838ec
https://hg.mozilla.org/mozilla-central/rev/de0ab3ea1796
https://hg.mozilla.org/mozilla-central/rev/179b81691182
https://hg.mozilla.org/mozilla-central/rev/c8b420c49253
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Comment 35•7 years ago
|
||
I'm getting a warning locally running jit-tests:
/home/jon/clone/modules/js/src/jit-test/tests/gc/bug-1155455.js: warning: unrecognized |jit-test| attribute skip-if !('gczeal' in this)
I don't know why this is happening for this test in particular.
Comment 37•7 years ago
|
||
This turned all of the tier3 jit tests orange on the pixel2 aarch64 tests:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&tier=1,2,3&searchStr=android-hw&group_state=expanded&revision=c8b420c49253532689f462813d7c9673ba798165
Was that expected? We previously had perma orange for jit{5,6,10} which prevented the tests from moving up to tier 2. Now, it is not apparent these tests are worth running in this state. Should I file a follow up bug for these new errors or to just disable the jit tests on pixel2 aarch64 altogether?
Updated•7 years ago
|
See Also: → arm64-baseline-tests
You need to log in
before you can comment on or make changes to this bug.
Description
•