Many stress tests fail with JIT disabled

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Tomas Popela]

Compiling JSC with:

$ build-webkit --jsc-only --release --system-malloc --no-jit

and running the stress suite with:

$ run-jsc-stress-tests --jsc $(pwd)/WebKitBuild/Release/bin/jsc --no-jit -v JSTests/stress

will make some of the tests fail with following:

Running stress/arith-abs-on-various-types.js.default
stress/arith-abs-on-various-types.js.default: Exception: The call without arguments should never exit.
stress/arith-abs-on-various-types.js.default: ERROR: Unexpected exit code: 3
FAIL: stress/arith-abs-on-various-types.js.default
Running stress/arith-abs-overflow.js.default
Running stress/arith-abs-to-arith-negate-range-optimizaton.js.default
stress/arith-abs-to-arith-negate-range-optimizaton.js.default: Exception: Failed optimizing testCheckedBetweenIntMinAndZeroExclusive(). None of the tested case need to OSR Exit.
stress/arith-abs-to-arith-negate-range-optimizaton.js.default: ERROR: Unexpected exit code: 3
FAIL: stress/arith-abs-to-arith-negate-range-optimizaton.js.default
...

What the failures have common is that they fail while testing the return value of numberOfDFGCompiles().

The problem is that when running with JIT disabled the return value is on purpose 1000000.0 - https://trac.webkit.org/browser/webkit/trunk/Source/JavaScriptCore/runtime/TestRunnerUtils.cpp?rev=212778#L77 .

We should skip these tests when JIT is disabled as they are not intended to work there.

[Tomas Popela]

Created attachment 333690 [details]
Patch

[Saam Barati]

Comment on attachment 333690 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=333690&action=review

> JSTests/stress/arith-abs-on-various-types.js:1
> +//@ skip if not $jitTests

I think this makes the line below not parsed. I think the stress tests harness only parses a single line. It’s worth verifying. If that’s right, you just want to make this entire line an if/else and combine in with the run type below

[Saam Barati]

(In reply to Saam Barati from comment #2)
> Comment on attachment 333690 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=333690&action=review
> 
> > JSTests/stress/arith-abs-on-various-types.js:1
> > +//@ skip if not $jitTests
> 
> I think this makes the line below not parsed. I think the stress tests
> harness only parses a single line. It’s worth verifying. If that’s right,
> you just want to make this entire line an if/else and combine in with the
> run type below

Never mind, I see you have a patch to make this work with skipping

[Saam Barati]

Comment on attachment 333690 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=333690&action=review

> JSTests/ChangeLog:9
> +        the return value of numberOfDFGCompiles(), which is always set to

Which other tests use this function? Maybe it should just return 0 instead of a really large number?

[Tomas Popela]

(In reply to Saam Barati from comment #4)
> Comment on attachment 333690 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=333690&action=review
> 
> > JSTests/ChangeLog:9
> > +        the return value of numberOfDFGCompiles(), which is always set to
> 
> Which other tests use this function? Maybe it should just return 0 instead
> of a really large number?

arith-abs-on-various-types.js
arith-abs-to-arith-negate-range-optimizaton.js
arith-acos-on-various-types.js
arith-acosh-on-various-types.js
arith-asin-on-various-types.js
arith-asinh-on-various-types.js
arith-atan-on-various-types.js
arith-atanh-on-various-types.js
arith-cbrt-on-various-types.js
arith-ceil-on-various-types.js
arith-clz32-on-various-types.js
arith-cos-on-various-types.js
arith-cosh-on-various-types.js
arith-expm1-on-various-types.js
arith-floor-on-various-types.js
arith-fround-on-various-types.js
arith-log-on-various-types.js
arith-log10-on-various-types.js
arith-log2-on-various-types.js
arith-negate-on-various-types.js
arith-round-on-various-types.js
arith-sin-on-various-types.js
arith-sinh-on-various-types.js
arith-sqrt-on-various-types.js
arith-tan-on-various-types.js
arith-tanh-on-various-types.js
arith-trunc-on-various-types.js
check-string-ident.js
compare-strict-eq-on-various-types.js
math-pow-coherency.js
pow-coherency.js
resources/standalone-pre.js
typedarray-intrinsic-getters-change-prototype.js

From these tests some use "<=":

0 $ git grep numberOfDFGCompiles | grep "<"
arith-ceil-on-various-types.js:146:        return numberOfDFGCompiles(testFunction) <= 2;
arith-ceil-on-various-types.js:148:    return numberOfDFGCompiles(testFunction) <= 1;
arith-floor-on-various-types.js:146:        return numberOfDFGCompiles(testFunction) <= 2;
arith-floor-on-various-types.js:148:    return numberOfDFGCompiles(testFunction) <= 1;
arith-negate-on-various-types.js:126:        return numberOfDFGCompiles(testFunction) <= 2;
arith-negate-on-various-types.js:128:    return numberOfDFGCompiles(testFunction) <= 1;
arith-round-on-various-types.js:146:        return numberOfDFGCompiles(testFunction) <= 2;
arith-round-on-various-types.js:148:    return numberOfDFGCompiles(testFunction) <= 1;
arith-trunc-on-various-types.js:146:        return numberOfDFGCompiles(testFunction) <= 2;
arith-trunc-on-various-types.js:148:    return numberOfDFGCompiles(testFunction) <= 1;
resources/standalone-pre.js:295:            if (testRunner.numberOfDFGCompiles(argument.f[i]) < numberOfCompiles)
resources/standalone-pre.js:299:        if (testRunner.numberOfDFGCompiles(argument.f) < numberOfCompiles)
typedarray-intrinsic-getters-change-prototype.js:21:    while(numberOfDFGCompiles(foo) < 1) {

[Saam Barati]

Comment on attachment 333690 [details]
Patch

r=me
It looks like this returns a large number because code like that while loop does:
while (#compiles < 1) ...

[Tomas Popela]

Comment on attachment 333690 [details]
Patch

Clearing flags on attachment: 333690

Committed r228513: <https://trac.webkit.org/changeset/228513>

[Tomas Popela]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/37571536>

[Tomas Popela]

Committed r230564: <https://trac.webkit.org/changeset/230564>