1603140 - Multi-value returns in baseline wasm compiler

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, enhancement, P2)

Description

[Andy Wingo [:wingo]]

The attached patch will start adding support for multi-value returns in the baseline wasm compiler.

Comment 1

[Andy Wingo [:wingo]]

Attachment: Bug 1603140 - Multi-value returns in baseline wasm compiler

This patch implements one half of support for multi-value returns in the
baseline compiler.

The idea is that when you have a function that has stack return values,
it will receive an additional synthetic argument: a pointer to the stack
results area. The layout of this area is the same as that understood by
BaseCompiler::pushBlockResults.

When compiling a function that has stack results, the stack result area
pointer will come in either in a register or on the stack. If it's on
the stack we record that offset and will load a pointer at that offset
when returning values. If the pointer is in a register, it gets spilled
to the stack after the spilled copies of previous register arguments.

When calling a function that has multiple return values, we bump the
stack pointer and pass an additional argument to the callee: the pointer
to that stack area. After the call, in the baseline compiler, it may be
that we need to shuffle any stack results down, because incoming
arguments are consumed by the call.

Note that support for actually returning multiple values is not yet
implemented in this patch; we're just adding infra. Also note that Ion
support isn't there yet either. However all tests pass, because no
FuncType that has more than one result gets past the validator.

Comment 2

[Andy Wingo [:wingo]]

Attachment: Bug 1603140 - Add multi-value return epilogue to baseline compiler

This commit enables multi-value returns in baseline-to-baseline calls.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69e62c1d0d53
Multi-value returns in baseline wasm compiler r=lth

Comment 4

[Natalia Csoregi [:nataliaCs]]

Backed out for bustage on WasmBaselineCompile.cpp

backout: https://hg.mozilla.org/integration/autoland/rev/e51924303b7b1ad280aca7eeded02b3fb690df4f

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=69e62c1d0d539a951f49e0ff967ae2392d161c2c&selectedJob=281382275

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=281382275&repo=autoland&lineNumber=5198

[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - In file included from Unified_cpp_js_src_wasm0.cpp:11:
[task 2019-12-16T16:09:25.107Z] 16:09:25 ERROR - /builds/worker/workspace/build/src/js/src/wasm/WasmBaselineCompile.cpp:1730:10: error: no matching member function for call to 'movePtr'
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - masm.movePtr(sp_, dest);
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - ^
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - /builds/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.h:733:8: note: candidate function not viable: no known conversion from 'js::jit::RegisterOrSP' to 'js::jit::Register' for 1st argument
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - void movePtr(Register src, Register dest) {
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - ^
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - /builds/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.h:736:8: note: candidate function not viable: no known conversion from 'js::jit::RegisterOrSP' to 'js::jit::ImmWord' for 1st argument
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - void movePtr(ImmWord imm, Register dest) {
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - ^
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - /builds/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.h:739:8: note: candidate function not viable: no known conversion from 'js::jit::RegisterOrSP' to 'js::jit::ImmPtr' for 1st argument
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - void movePtr(ImmPtr imm, Register dest) {
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - ^
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - /builds/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.h:742:8: note: candidate function not viable: no known conversion from 'js::jit::RegisterOrSP' to 'wasm::SymbolicAddress' for 1st argument
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - void movePtr(wasm::SymbolicAddress imm, Register dest) {
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - ^
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - /builds/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.h:746:8: note: candidate function not viable: no known conversion from 'js::jit::RegisterOrSP' to 'js::jit::ImmGCPtr' for 1st argument
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - void movePtr(ImmGCPtr imm, Register dest) {
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - ^
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - /builds/worker/workspace/build/src/js/src/jit/MacroAssembler.h:3030:8: note: candidate function not viable: no known conversion from 'js::jit::RegisterOrSP' to 'js::jit::TrampolinePtr' for 1st argument
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - void movePtr(TrampolinePtr ptr, Register dest) {
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - ^
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - 1 error generated.
[task 2019-12-16T16:09:25.107Z] 16:09:25 ERROR - make[4]: *** [/builds/worker/workspace/build/src/config/rules.mk:738: Unified_cpp_js_src_wasm0.o] Error 1
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/js/src/wasm'
[task 2019-12-16T16:09:25.107Z] 16:09:25 ERROR - make[3]: *** [/builds/worker/workspace/build/src/config/recurse.mk:74: js/src/wasm/target-objects] Error 2
[task 2019-12-16T16:09:25.107Z] 16:09:25 INFO - make[3]: *** Waiting for unfinished jobs....

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:wingo, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 6

[Andy Wingo [:wingo]]

Thanks for the landing and sheriffing, and apologies for the delay as I was on holidays. These patches do not need to land before the release, so I will hold off on relanding until after the branch.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/63f89c300859
Multi-value returns in baseline wasm compiler r=lth

Comment 8

[Andrei Ciure[:aciure]]

Backed out changeset 63f89c300859 (bug 1603140) for causing wasm related bustages

push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=283638798&resultStatus=testfailed%2Cbusted%2Cexception%2Cusercancel%2Crunnable&searchStr=linux%2Cx64%2Cdebug%2Cspidermonkey%2Cbuilds%2Cspidermonkey-sm-nonunified-linux64%2Fdebug%2Csm%28nu%29&revision=63f89c30085907e524cc8694cba029b9012f32e9

backout: https://hg.mozilla.org/integration/autoland/rev/33d7640d6bad33e6f0401f9f5ef8ea8946780b9d

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/28e56e8fe2bb
Multi-value returns in baseline wasm compiler r=lth

Comment 10

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/28e56e8fe2bb

Comment 11

[Andy Wingo [:wingo]]

Reopening as there are patches yet to land. (Is there a phab flag to set to prevent the bug from being closed?)

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/14c9f19d716c
Add multi-value return epilogue to baseline compiler r=lth

Comment 13

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/14c9f19d716c

Comment 14

[Lars T Hansen [:lth]]

(In reply to Andy Wingo [:wingo] from comment #11)

Reopening as there are patches yet to land. (Is there a phab flag to set to prevent the bug from being closed?)

You need to set the leave-open keyword on the bugzilla bug.