wasm/limits.js jit-test requires shared memory, which breaks Cranelift/aarch64 WebAssembly tests
Categories
(Core :: JavaScript: WebAssembly, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox81 | --- | fixed |
People
(Reporter: cfallin, Assigned: jseward)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
In Bug 1606624, it seems that the wasm/limits.js
jit-test was created (or merged from other tests) in such a way that it requires shared-memory support to pass tests. This breaks Cranelift WebAssembly jit-tests on aarch64.
We can temporarily disable the whole wasm/limits.js
test until we have shared memory support on Cranelift/aarch64 (Bug 1649927), but ideally we would actually split out the shared-memory portions of the test and disable those.
Separately, we need to establish CI on aarch64 to ensure that this doesn't go unnoticed in the future. (We had an initial conversation about this ~3 weeks ago and it will be tracked by Bug 1649928.)
Updated•4 years ago
|
Comment 1•4 years ago
|
||
The severity field is not set for this bug.
:lth, could you have a look please?
For more information, please visit auto_nag documentation.
Comment 2•4 years ago
|
||
Lars is on holiday. Setting S4 as it is not something our users face at the moment (Cranelift on aarch64, which is Nightly only, pref'd off by default).
Updated•4 years ago
|
Comment 3•4 years ago
|
||
Chris, steps to reproduce? I don't seem to be able to. I have an arm64 simulator build; I have removed the guard in the test file; I have added --test-also=--wasm-compiler=cranelift to directives.txt; I have verified that that's one of the configs being run. But I'm not seeing any error.
Reporter | ||
Comment 4•4 years ago
|
||
We disabled this test on Cranelift for now, but the error should reappear if one disables this directive:
https://searchfox.org/mozilla-central/source/js/src/jit-test/tests/wasm/limits.js#1
However, :jseward is getting close to landing Wasm atomics in the new aarch64 backend, so I think that we can probably just wait for that to happen in order to resolve this issue (the timeline for that wasn't as clear when I created this bug), unless we think it's important to have this test active in the meantime.
Comment 5•4 years ago
|
||
The problem was really (see comment 3 about "the guard in the test file") that I removed the directive, and still could not reproduce. Happy to not worry further though :-)
Reporter | ||
Comment 6•4 years ago
|
||
Oh, sorry, I'm clearly in need of more caffeine. On re-examination, it's likely that Cranelift wasn't selected because there's no --shared-memory=off
(unless that's just omitted here?). Anyway, yes, no need to burn more time on this. Thanks for taking a look!
Comment 7•4 years ago
|
||
That makes sense, I should have considered that!
Reporter | ||
Comment 8•4 years ago
|
||
This patch re-enables the wasm/limits.js
test, which we had previously
disabled on Cranelift because it depended on shared memory. Now that the
Cranelift/aarch64 Wasm backend supports shared memory, there is no
reason to exclude this test any more.
Comment 10•4 years ago
|
||
bugherder |
Description
•