Add anyref-related instructions to Ion
Categories
(Core :: JavaScript: WebAssembly, enhancement, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox67 | --- | fixed |
People
(Reporter: lth, Assigned: lth)
References
Details
Attachments
(5 files, 8 obsolete files)
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Comment 1•7 years ago
|
||
Assignee | ||
Comment 3•7 years ago
|
||
Assignee | ||
Comment 4•7 years ago
|
||
Assignee | ||
Comment 5•7 years ago
|
||
Rebased and with some more fixes. I get 12 failures in a debug jit-test run, 10 in a release run. These are all believed to be caused by missing tracing support, since we no longer have the suppressGC hack in place.
Assignee | ||
Comment 6•7 years ago
|
||
Fix a bug in Ion code generation for table.get: the return value from the instance method is not the anyref, but a raw pointer to a nonmoveable location that contains the anyref, or a null pointer signifying an error. In addition, the code that was there used AnyRef for the return value from the call but that needs to be MIRType::Pointer.
This patch is not completely clean (some formatting, some naming) but seems to be OK. With this applied: 6 errors in wasm/gc in a debug build and --no-wasm-baseline.
Assignee | ||
Comment 7•7 years ago
|
||
Fix a bug in select on x64: we inadvertently got a 32-bit move. We don't necessarily want to land this patch as-is since it has a platform ifdef, but it'll be an OK placeholder.
Comment 8•6 years ago
|
||
Here's a revised, resequenced and rebased patchset, dependent on bug 1520478,
which introduces MIRType::RefOrNull for baseline. It incorporates the
tableset and select bug fixes, and further RefOrNull changes as required:
bug1508559-v6-1-Tgrow-fill-arg.diff
bug1508559-v6-2-ion-insns.diff
bug1508559-v6-3-factor-rabaldr.diff
bug1508559-v6-4-reorganize-baldr.diff
bug1508559-v6-5-enable-ion-gc.diff
bug1508559-v6-6-change-directives.diff
All prefixes of the first four (v6-1 to v6-4) build and test OK. They
introduce new functionality but don't enable it. v6-5 enables the
functionality and so causes a couple of failures due to missing GC support.
v6-6 enables tests and causes further failures at present.
Comment 9•6 years ago
|
||
New patchset, based on m-c of 459338:1ba05f897328 (Fri Feb 15 13:50:07 2019
+0900)
First apply bug1527563-simplify-calls-3.diff, from that bug. Then the tarfile
contents, in order, bug1508559-v6-[1..6]*.diff.
Comment 10•6 years ago
|
||
Applies directly to m-c 459716:3d6a4067fd12. No other patches needed.
Assignee | ||
Comment 11•6 years ago
|
||
Factor common wasm write barrier functionality out of the baseline
compiler so that it can also be used by Ion.
Assignee | ||
Comment 12•6 years ago
|
||
We reorganize the nodes for handling global variable access so as to
allow for more shared code paths and to allow for write barriers on
pointer stores.
For pointer stores, the store is split in two: one part to compute the
address of the slot to be stored (direct or indirect) and one part to
perform the store. The address computation is different for indirect
and direct globals, but the ultimate loading and storing of the
global's value can then be handled by the same code in both cases.
For all loads, and for non-pointer stores, the LIR and code generation
paths have been merged into WasmLoadSlot/WasmStoreSlot nodes.
Assignee | ||
Comment 13•6 years ago
|
||
This bundle off of m-i 459842:7fc33c7516c3 has the two patches that are up for review on this bug, plus the rebased and somewhat reengineered reftypes-for-ion patch, plus a couple of patches to enable reftypes for ion and trigger more test cases.
(It obsoletes the tar file that is on this bug, but I'll not hide that yet.)
Comment 14•6 years ago
|
||
Comment 15•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 16•6 years ago
|
||
Add support to WasmIonCompile for the instructions in the reftypes
proposal: ref.null, ref.is_null, table.get, table.set, table.grow,
table.size.
Also add support for the ref.eq instruction from the gc proposal.
Also update the test suite so that we will not ion-compile test cases
that use gc features that are not landed here.
Note that this patch does not change the compiler-selection behavior:
If --wasm-gc is enabled then only the baseline compiler will be used;
if --wasm-gc is not enabled then no compiler will recognize these
opcodes. Enabling Ion for reftypes content is the subject of
subsequent work.
Comment 17•6 years ago
|
||
bugherder |
Comment 18•6 years ago
|
||
Assignee | ||
Comment 19•6 years ago
|
||
Julian, re the ion patch, notice the introduction of MWasmLoadRef, which loads a RefOrNull via a raw pointer. This parallels in many ways MWasmStoreRef, already introduced for the global variables patch, but the global variables code does not use MWasmLoadRef because that's not been necessary. But if it simplifies anything for the work you're doing on the stack maps there is no reason we could not change that.
Comment 20•6 years ago
|
||
Assignee | ||
Comment 21•6 years ago
|
||
I'm going to move the patch that enables ion for reftypes content to another bug, possibly a new one.
Comment 22•6 years ago
|
||
Backed out changeset 386a2afe9189 (Bug 1508559) for failing raptor tests in /home/cltbld/tasks/task_1550758926/build/raptor.json CLOSED TREE
14:39:53 INFO - raptor-control-server received webext_status: __raptor_shutdownBrowser
14:39:53 INFO - raptor-control-server shutting down browser (pid: 3604)
14:39:53 INFO - 127.0.0.1 - - [21/Feb/2019 06:39:53] "POST / HTTP/1.1" 200 -
14:39:53 INFO - raptor-control-server received webext_status: Removed tab 3
14:39:53 INFO - PID 3604 | [Parent 3604, Gecko_IOThread] WARNING: pipe error (132): Connection reset by peer: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 357
14:40:09 INFO - raptor-main removing webext /home/cltbld/tasks/task_1550758926/build/tests/raptor/webext/raptor
14:40:09 INFO - results-handler summarizing raptor test results
14:40:09 INFO - raptor-output error: no raptor test results found for raptor-wasm-godot-firefox
14:40:09 INFO - raptor-output error: no summarized raptor results found for raptor-wasm-godot-firefox
14:40:09 INFO - raptor-control-server shutting down control server
14:40:10 INFO - raptor-main finished
14:40:10 INFO - raptor-main TEST-UNEXPECTED-FAIL: no raptor test results were found for raptor-wasm-godot-firefox
14:40:10 ERROR - Return code: 1
14:40:10 WARNING - setting return code to 1
14:40:10 CRITICAL - PERFHERDER_DATA was seen 0 times, expected 1.
14:40:10 INFO - copying raptor results to upload dir:
14:40:10 INFO - /home/cltbld/tasks/task_1550758926/build/blobber_upload_dir/perfherder-data.json
14:40:10 INFO - copying raptor results from /home/cltbld/tasks/task_1550758926/build/raptor.json to /home/cltbld/tasks/task_1550758926/build/blobber_upload_dir/perfherder-data.json
14:40:10 CRITICAL - Error copying results /home/cltbld/tasks/task_1550758926/build/raptor.json to upload dir /home/cltbld/tasks/task_1550758926/build/blobber_upload_dir/perfherder-data.json
14:40:10 INFO - [Errno 2] No such file or directory: u'/home/cltbld/tasks/task_1550758926/build/raptor.json'
14:40:10 INFO - Running post-action listener: _package_coverage_data
14:40:10 INFO - Running post-action listener: _resource_record_post_action
14:40:10 INFO - Running post-action listener: process_java_coverage_data
14:40:10 INFO - Running post-action listener: stop_device
14:40:10 INFO - [mozharness: 2019-02-21 14:40:10.096866Z] Finished run-tests step (success)
14:40:10 INFO - Running post-run listener: _resource_record_post_run
14:40:10 INFO - Total resource usage - Wall time: 183s; CPU: 3.0%; Read bytes: 97591296; Write bytes: 606150656; Read time: 768; Write time: 42840
14:40:10 INFO - TinderboxPrint: CPU usage<br/>3.2%
14:40:10 INFO - TinderboxPrint: I/O read bytes / time<br/>97,591,296 / 768
14:40:10 INFO - TinderboxPrint: I/O write bytes / time<br/>606,150,656 / 42,840
14:40:10 INFO - TinderboxPrint: CPU idle<br/>1,409.6 (96.7%)
14:40:10 INFO - TinderboxPrint: CPU user<br/>38.1 (2.6%)
14:40:10 INFO - TinderboxPrint: Swap in / out<br/>0 / 0
14:40:10 INFO - install - Wall time: 14s; CPU: 13.0%; Read bytes: 0; Write bytes: 2146304; Read time: 0; Write time: 16
14:40:10 INFO - run-tests - Wall time: 170s; CPU: 2.0%; Read bytes: 94773248; Write bytes: 602587136; Read time: 768; Write time: 42784
14:40:10 WARNING - returning nonzero exit status 1
[taskcluster 2019-02-21T14:40:10.188Z] Exit Code: 1
[taskcluster 2019-02-21T14:40:10.188Z] User Time: 52.728s
[taskcluster 2019-02-21T14:40:10.188Z] Kernel Time: 7.044s
[taskcluster 2019-02-21T14:40:10.188Z] Wall Time: 3m39.253793816s
[taskcluster 2019-02-21T14:40:10.188Z] Result: FAILED
[taskcluster 2019-02-21T14:40:10.188Z] === Task Finished ===
[taskcluster 2019-02-21T14:40:10.188Z] Task Duration: 3m39.254002624s
[taskcluster 2019-02-21T14:40:10.555Z] Uploading artifact public/logs/localconfig.json from file logs/localconfig.json with content encoding "gzip", mime type "application/json" and expiry 2020-02-21T13:13:55.653Z
[taskcluster 2019-02-21T14:40:11.286Z] Uploading artifact public/test_info/resource-usage.json from file build/blobber_upload_dir/resource-usage.json with content encoding "gzip", mime type "application/json" and expiry 2020-02-21T13:13:55.653Z
[taskcluster:error] exit status 1
Comment 23•6 years ago
|
||
Comment 24•6 years ago
|
||
bugherder |
Assignee | ||
Comment 25•6 years ago
•
|
||
This was backed out for failure, presumably related to this msg earlier in the log:
JavaScript error: http://localhost:55780/wasm-godot/godot.js line 1 >
WebAssembly.instantiate, line 334514: RuntimeError: index out of bounds
Assignee | ||
Comment 26•6 years ago
|
||
I can repro on a plain release build on the try server (no webrender or other special sauce), but not with a release build built locally. https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=230302571&repo=try&lineNumber=569
The error message references a completely unlikely line number:
JavaScript error: http://localhost:45230/wasm-godot/godot.js line 1 >
WebAssembly.instantiate, line 1199265: RuntimeError: index out of bounds
Even if that's a wasm instruction offset it's very different from the one in the error message in comment 25.
Assignee | ||
Comment 27•6 years ago
|
||
Haha, reproduces when asking specifically for Ion:
./mach raptor-test -t raptor-wasm-godot-ion
All is well with the world.
Assignee | ||
Comment 28•6 years ago
|
||
Error comes from a trap in compiled code, not from any of the changed table/memory initialization code.
Assignee | ||
Comment 29•6 years ago
|
||
Incorrect platform support for wasmSelect that affected only Ion - I simplified a conditional move too much.
Updated•6 years ago
|
Comment 30•6 years ago
|
||
Comment 31•6 years ago
|
||
bugherder |
Description
•