Bump Cranelift to 152e31796970262068b4fe93e10395385bf9218c
Categories
(Core :: JavaScript: WebAssembly, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox73 | --- | fixed |
People
(Reporter: bbouvier, Assigned: bbouvier)
References
Details
Attachments
(3 files)
Assignee | ||
Comment 1•4 years ago
|
||
Failures when running the test suite with Spidermonkey:
Related to bulk memory operators:
wasm/spec/bulk.wast.js
--test-wasm-await-tier2 wasm/spec/bulk.wast.js
--disable-wasm-huge-memory wasm/spec/bulk.wast.js
wasm/spec/memory_fill.wast.js
wasm/spec/memory_copy.wast.js
--disable-wasm-huge-memory wasm/spec/memory_fill.wast.js
--test-wasm-await-tier2 wasm/spec/memory_fill.wast.js
--disable-wasm-huge-memory wasm/spec/memory_copy.wast.js
--test-wasm-await-tier2 wasm/spec/memory_copy.wast.js
--disable-wasm-huge-memory wasm/spec/memory_init.wast.js
wasm/spec/memory_init.wast.js
--test-wasm-await-tier2 wasm/spec/memory_init.wast.js
wasm/spec/table_copy.wast.js
--test-wasm-await-tier2 wasm/spec/table_copy.wast.js
--disable-wasm-huge-memory wasm/spec/table_copy.wast.js
--test-wasm-await-tier2 wasm/spec/table_init.wast.js
--disable-wasm-huge-memory wasm/spec/table_init.wast.js
wasm/spec/table_init.wast.js
Other failures (may require further investigation):
wasm/spec/type.wast.js
--disable-wasm-huge-memory wasm/spec/type.wast.js
--test-wasm-await-tier2 wasm/spec/type.wast.js
Assignee | ||
Comment 2•4 years ago
|
||
Assignee | ||
Comment 3•4 years ago
|
||
Updated•4 years ago
|
Comment 4•4 years ago
|
||
The issue with type.wast is that multi-value is disabled when cranelift is enabled, so there are expected failures around types with multiple return values. But the validation code wouldn't check if cranelift was enabled, only multi-value, so the tests would pass. Updating the validation code to not allow multi-value with cranelift led to more failures.
The simplest option I saw was to just enabled multi-value with cranelift, as IIUC there is now support. This resolved all the test failures (besides bulk-memory).
Assignee | ||
Comment 5•4 years ago
|
||
The simplest option I saw was to just enabled multi-value with cranelift, as IIUC there is now support. This resolved all the test failures (besides bulk-memory).
There's missing glue between Cranelift and Spidermonkey to have proper support for multi value types in wasm, so I expect this won't work at runtime.
The right fix is to have the Validate function consider if multi values can be enabled (by adding another parameter to CompilerEnvironment) and see in which places we need to check for this; probably when decoding the type section and in misc places in the code section.
Let get this done in a follow-up.
Assignee | ||
Comment 6•4 years ago
|
||
This hasn't landed yet, and there are new test failures that were not there last week:
wasm/import-export.js
/home/ben/code/mozilla-inbound/js/src/jit-test/lib/asserts.js:72:23 Error: Assertion failed: expected exception LinkError, got RuntimeError: index out of bounds
Stack:
assertErrorMessage@/home/ben/code/mozilla-inbound/js/src/jit-test/lib/asserts.js:72:23
assertSegmentFitError@/home/ben/code/mozilla-inbound/js/src/jit-test/tests/wasm/import-export.js:22:27
@/home/ben/code/mozilla-inbound/js/src/jit-test/tests/wasm/import-export.js:498:22
wasm/tables.js
/home/ben/code/mozilla-inbound/js/src/jit-test/lib/asserts.js:72:23 Error: Assertion failed: expected exception LinkError, got RuntimeError: index out of bounds
Stack:
assertErrorMessage@/home/ben/code/mozilla-inbound/js/src/jit-test/lib/asserts.js:72:23
assertSegmentFitError@/home/ben/code/mozilla-inbound/js/src/jit-test/tests/wasm/tables.js:14:27
@/home/ben/code/mozilla-inbound/js/src/jit-test/tests/wasm/tables.js:25:22
Probably the same underlying issue.
Assignee | ||
Comment 7•4 years ago
|
||
This is due to https://phabricator.services.mozilla.com/D54598, which introduced assertion methods based on bulk-memory being properly implemented, so maybe the right path forward is to review and land bulk mem in Cranelift...
Updated•4 years ago
|
Assignee | ||
Comment 8•4 years ago
|
||
The eagerBoundsCheck variable was desynchronized from compiler feature support
for bulk memory operations, namely for Cranelift. This manually checks for
Cranelift usage, and this will need to be reverted once bulk memory opcodes
support land in Cranelift.
Pushed by bbouvier@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6c803ec19d69 Bump cranelift to 152e317969702620; r=rhunt https://hg.mozilla.org/integration/autoland/rev/aa13508625cf Output of mach rust vendor for the Cranelift bump; r=rhunt https://hg.mozilla.org/integration/autoland/rev/827afbfc6f34 Tweak eager segments bound checks behavior for Cranelift; r=rhunt
Comment 10•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6c803ec19d69
https://hg.mozilla.org/mozilla-central/rev/aa13508625cf
https://hg.mozilla.org/mozilla-central/rev/827afbfc6f34
Description
•