1523015 - Allow ARM64 Ion to be toggled by pref

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P2)

Description

[Sean Stangl [:sstangl]]

Attachment: 0001-Enable-Ion-on-ARM64-but-disable-in-browser-by-pref.-.patch

The attached patch removes the code that hard-disables IonMonkey for ARM64. With this patch applied:

1. Shell builds will run with Ion enabled by default. This includes on TBPL, where new failures will occur. I believe this is acceptable because there are already a few Baseline failures, the number of new IonMonkey failures is low (double-digits), and we are actively working on fixing them.

2. Browser builds will run with Ion disabled by default, but it can be enabled manually by using about:config to change javascript.options.ion to boolean "true".

@nbp, this was requested by Steve, forwarding the requests of other managers. The hope is that we're in a state where things aren't too crashy, and so they can at least view the behavior personally on a subset of webpages.

Things like "unsigned division" still crash, but I'll fix that up next week.

Comment 1

[Sean Stangl [:sstangl]]

Attachment: 0001-Bug-1523015-Enable-Ion-on-ARM64-but-disable-in-brows.patch

v2: Same as the other patch, but I just realized that if we're enabling Ion for shell, we might as well enable the tests also, since they pass.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0121f2af5821
Enable Ion on ARM64, but disable in-browser by pref. r=nbp

Comment 3

[Cristina Coroiu [:ccoroiu]]

Backed out changeset 0121f2af5821 (bug 1523015) for spidermonkey failures at js/src/jit-test/tests/atomics/mutual-exclusion.js

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/67d7ef194fbee7c5925ef1d67c310e5257c188a3

Failure push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&selectedJob=224485151&revision=0121f2af5821a32deb329f1acf7e1ad4d7787560

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=224485151&repo=mozilla-inbound&lineNumber=4226

[task 2019-01-28T16:59:06.174Z] TEST-PASS | js/src/jit-test/tests/auto-regress/bug1183241.js | Success (code 3, args "") [0.2 s]
[task 2019-01-28T16:59:06.175Z] {"action": "test_start", "jitflags": "", "pid": 12958, "source": "jittests", "test": "auto-regress/bug1183241.js", "thread": "main", "time": 1548694746.004868}
[task 2019-01-28T16:59:06.176Z] {"action": "test_end", "extra": {"jitflags": "", "pid": 12958}, "jitflags": "", "message": "Success", "pid": 12958, "source": "jittests", "status": "PASS", "test": "auto-regress/bug1183241.js", "thread": "main", "time": 1548694746.174743}
[task 2019-01-28T16:59:06.190Z] Limit = 1
[task 2019-01-28T16:59:06.192Z] Hit MOZ_CRASH(NYI) at /builds/worker/workspace/build/src/js/src/jit/arm64/CodeGenerator-arm64.cpp:1734
[task 2019-01-28T16:59:06.193Z] Exit code: -11
[task 2019-01-28T16:59:06.194Z] FAIL - atomics/mutual-exclusion.js
[task 2019-01-28T16:59:06.195Z] TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/atomics/mutual-exclusion.js | Hit MOZ_CRASH(NYI) at /builds/worker/workspace/build/src/js/src/jit/arm64/CodeGenerator-arm64.cpp:1734 (code -11, args "") [0.3 s]
[task 2019-01-28T16:59:06.195Z] {"action": "test_start", "jitflags": "", "pid": 12909, "source": "jittests", "test": "atomics/mutual-exclusion.js", "thread": "main", "time": 1548694745.928952}
[task 2019-01-28T16:59:06.196Z] {"action": "test_end", "extra": {"jitflags": "", "pid": 12909}, "jitflags": "", "message": "Hit MOZ_CRASH(NYI) at /builds/worker/workspace/build/src/js/src/jit/arm64/CodeGenerator-arm64.cpp:1734", "pid": 12909, "source": "jittests", "status": "FAIL", "test": "atomics/mutual-exclusion.js", "thread": "main", "time": 1548694746.190998}
[task 2019-01-28T16:59:06.197Z] INFO exit-status : -11
[task 2019-01-28T16:59:06.198Z] INFO timed-out : False
[task 2019-01-28T16:59:06.199Z] INFO stdout > Limit = 1
[task 2019-01-28T16:59:06.200Z] INFO stderr 2> Hit MOZ_CRASH(NYI) at /builds/worker/workspace/build/src/js/src/jit/arm64/CodeGenerator-arm64.cpp:1734
[task 2019-01-28T16:59:06.201Z] TEST-PASS | js/src/jit-test/tests/atomics/store-does-not-truncate-returnval.js | Success (code 0, args "") [0.2 s]
[task 2019-01-28T16:59:06.201Z] {"action": "test_start", "jitflags": "", "pid": 12947, "source": "jittests", "test": "atomics/store-does-not-truncate-returnval.js", "thread": "main", "time": 1548694745.985358}
[task 2019-01-28T16:59:06.201Z] {"action": "test_end", "extra": {"jitflags": "", "pid": 12947}, "jitflags": "", "message": "Success", "pid": 12947, "source": "jittests", "status": "PASS", "test": "atomics/store-does-not-truncate-returnval.js", "thread": "main", "time": 1548694746.191292}
[task 2019-01-28T16:59:06.277Z] TEST-PASS | js/src/jit-test/tests/auto-regress/bug1147907.js | Success (code 0, args "") [0.3 s]
[task 2019-01-28T16:59:06.277Z] {"action": "test_start", "jitflags": "", "pid": 12957, "source": "jittests", "test": "auto-regress/bug1147907.js", "thread": "main", "time": 1548694745.999065}
[task 2019-01-28T16:59:06.277Z] {"action": "test_end", "extra": {"jitflags": "", "pid": 12957}, "jitflags": "", "message": "Success", "pid": 12957, "source": "jittests", "status": "PASS", "test": "auto-regress/bug1147907.js", "thread": "main", "time": 1548694746.274499}
[task 2019-01-28T16:59:06.316Z] Hit MOZ_CRASH(NYI) at /builds/worker/workspace/build/src/js/src/jit/arm64/Lowering-arm64.cpp:381
[task 2019-01-28T16:59:06.316Z] Exit code: -11
[task 2019-01-28T16:59:06.316Z] FAIL - atomics/basic-tests.js
[task 2019-01-28T16:59:06.316Z] TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/atomics/basic-tests.js | Hit MOZ_CRASH(NYI) at /builds/worker/workspace/build/src/js/src/jit/arm64/Lowering-arm64.cpp:381 (code -11, args "") [0.4 s]
[task 2019-01-28T16:59:06.316Z] {"action": "test_start", "jitflags": "", "pid": 12907, "source": "jittests", "test": "atomics/basic-tests.js", "thread": "main", "time": 1548694745.9149501}
[task 2019-01-28T16:59:06.316Z] {"action": "test_end", "extra": {"jitflags": "", "pid": 12907}, "jitflags": "", "message": "Hit MOZ_CRASH(NYI) at /builds/worker/workspace/build/src/js/src/jit/arm64/Lowering-arm64.cpp:381", "pid": 12907, "source": "jittests", "status": "FAIL", "test": "atomics/basic-tests.js", "thread": "main", "time": 1548694746.314398}
[task 2019-01-28T16:59:06.316Z] INFO exit-status : -11
[task 2019-01-28T16:59:06.316Z] INFO timed-out : False

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

There are also jit failures on Android AArch: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=224493440&repo=mozilla-inbound&lineNumber=1983

17:42:18 INFO - TEST-PASS | tests/jit-test/jit-test/tests/bug828119.js | Success (code 0, args "--no-baseline --no-ion --more-compartments") [0.2 s]
17:42:18 INFO - {"action": "test_start", "jitflags": "--no-baseline --no-ion --more-compartments", "pid": 429, "source": "jittests", "test": "bug828119.js", "thread": "main", "time": 1548697338.6115952}
17:42:18 INFO - {"action": "test_end", "extra": {"jitflags": "--no-baseline --no-ion --more-compartments"}, "jitflags": "--no-baseline --no-ion --more-compartments", "message": "Success", "pid": 429, "source": "jittests", "status": "PASS", "test": "bug828119.js", "thread": "main", "time": 1548697338.816862}
17:42:19 INFO - exception output: Assertion failed: bad label: 2:
17:42:19 INFO - abort@/data/local/tests/jit-tests/jit-tests/tests/bug830943.js:32:34
17:42:19 INFO - assert@/data/local/tests/jit-tests/jit-tests/tests/bug830943.js:37:9
17:42:19 INFO - _main@/data/local/tests/jit-tests/jit-tests/tests/bug830943.js:98:13
17:42:19 INFO - callMain@/data/local/tests/jit-tests/jit-tests/tests/bug830943.js:255:12
17:42:19 INFO - doRun@/data/local/tests/jit-tests/jit-tests/tests/bug830943.js:261:19
17:42:19 INFO - run@/data/local/tests/jit-tests/jit-tests/tests/bug830943.js:265:16
17:42:19 INFO - @/data/local/tests/jit-tests/jit-tests/tests/bug830943.js:268:1
17:42:19 INFO - uncaught exception: Assertion: Assertion failed: bad label: 2
17:42:19 INFO - (Unable to print stack trace)

Comment 5

[Nicolas B. Pierron [:nbp]]

(In reply to Cristina Coroiu [:ccoroiu] from comment #3)

Backed out changeset 0121f2af5821 (bug 1523015) for spidermonkey failures at js/src/jit-test/tests/atomics/mutual-exclusion.js

I think, as written by Sean, this is the whole point of enabling this feature sooner, is to keep track of these issues on our CI without having a special build.

(In reply to Sean Stangl [:sstangl] from comment #0)

@nbp, this was requested by Steve, forwarding the requests of other managers. The hope is that we're in a state where things aren't too crashy, and so they can at least view the behavior personally on a subset of webpages.

ni? Steve, to see if we can find a solution to keep track of these crashes on CI even if we know that they are a few expected crashes that we are going to fix in the upcoming weeks.

Comment 6

[Steven DeTar [:sdetar]]

Sean said this might not be possible now, if that is the case we will just need to wait a coupe weeks until we at the point these crashes are not happening.

Comment 7

[Sean Stangl [:sstangl]]

Attachment: Summary: Enable Ion on ARM64, but disable in-browser by pref.

Comment 8

[Sean Stangl [:sstangl]]

Submitted a try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ab6b9baf48ef27e91e3bc98515c7650ce3950bcb

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1b9f4cb229fa
Summary: Enable Ion on ARM64, but disable in-browser by pref. r=nbp

Comment 10

[Andreea Pavel [:apavel]]

Backed out for arm64 bustages

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=1b9f4cb229fa56bae73b902dbf454e27ceaa0dc0&selectedJob=228003097

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=228003097&repo=autoland&lineNumber=21871

Backout: https://hg.mozilla.org/integration/autoland/rev/1186e4e5160a0d5bd869bc6b473171faa17d474c

This also had the following tier2 perma failure https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=228009836&repo=autoland&lineNumber=5561

Comment 11

[Sean Stangl [:sstangl]]

The failing jsapi-test contains the comment "Disable the JIT because if we don't this test fails. See bug 1160414."

This patch keeps the jit-tests disabled in both the shell and in the browser, so I don't see why that should change. Will require investigation.

Comment 12

[Sean Stangl [:sstangl]]

Ah, the test accidentally turns on Ion, even though it should be disabled.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2b3012fa3cbf
Summary: Enable Ion on ARM64, but disable in-browser by pref. r=nbp

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2b3012fa3cbf

Comment 15

[Ionuț Goldan [:igoldan]]

Looks like this brought big Speedometer improvements on AARCH64! \0/

== Change summary for alert #19341 (as of Tue, 12 Feb 2019 21:00:25 GMT) ==

Improvements:

14% raptor-speedometer-geckoview android-hw-p2-8-0-android-aarch64 opt 18.22 -> 20.75

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=19341

Comment 16

[Sean Stangl [:sstangl]]

It shouldn't have... it's supposed to be disabled!

Comment 17

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Is there anything that needs to happen here for 66?

Comment 18

[Sean Stangl [:sstangl]]

Liz, yes -- in Bug 1528621.

Comment 19

[Mike Hommey [:glandium]]

huh? this didn't land on 66, so bug 1528621 shouldn't be needed there

Comment 20

[Mike Hommey [:glandium]]

Am I missing something in comment 19?

Comment 21

[Sean Stangl [:sstangl]]

Oh, sorry, I didn't parse the 66 correctly. The pref wasn't enabled for 66.