2002625 - Remove CallOnMainStack

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, task, P1)

Description

[Ryan Hunt [:rhunt]]

It'd be really nice if we had the invariant that only wasm code runs on a suspendable stack (with maybe a couple limited exceptions for performance reasons). Right now we have a bunch of native frames that can run on the suspendable stack. These sometimes will use CallOnMainStack to switch to the main stack before performing more work, but not always.

I have patches which update all of our wasm exits (import exit, trap stub, debug stub, and throw unwinder) to be suspendable stack aware and switch to the main stack before re-entering native code. This eliminates the need for CallOnMainStack and gives us a tighter invariant around the main stack and VM code.

Comment 1

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - wasm: Merge SuspenderContext and wasm::Context. r?yury

We don't gain much by having SuspenderContext separate from
wasm::Context, and will make future patches harder. This
patch merges them.

One side effect is that SuspenderObject is now a public
interface in WasmPI.h. This is required for other
changes and isn't unusual.

Comment 2

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - wasm: Move activeSuspender updating into wasm::Context. r?yury

I'd like to move all the logic for updating the VM state when a
suspender changes into a single location. wasm::Context makes the
most sense to me.

Comment 3

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - wasm: Remove onSuspendableStack flag. r?yury

We can just use activeSuspender for this and reduce the
amount of state we need to update.

Comment 4

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - wasm: Move Win32 TIB updating into wasm::Context. r?yury

Continue moving more logic into a single place in
wasm::Context.

Comment 5

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - wasm: Move arbitrary code assertions into wasm::Context. r?yury

Move more suspender switching logic into wasm::Context.

Comment 6

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - wasm: Store a set of all suspenders, not just suspended. r?yury

This simplifies the logic when switching stacks to no longer
have to juggle a linked list. Suspender's also no longer need
to be linked list elements. It also removes the need to track
the suspender context for a given suspender.

Comment 7

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - wasm: Drop suspendedBy field and use SuspenderState for this. r?yury

Also add assertions to accessors around current state.

Comment 8

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - wasm: Merge SuspenderObject and SuspenderObjectData. r?yury

Now that we don't require a SuspenderObjectData to be a linked list
element, we can merge it with SuspenderObject. This reduces the number
of registers we need to for operations, and indirections to use.

The JIT routines are largely untouched and can be simplified in a
future patch.

Comment 9

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - wasm: Simplify codegen after merging data and suspender. r?yury

Now that they are actually the same thing, simplify their codegen.

Comment 10

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - wasm: Dynamically switch to main stack in builtin thunks. r?yury

Now that the sequence of operations performed in a CallOnMain
operation have been simplified, we can perform them relatively
easily in JIT code.

Extend the Exit prologue and epilogues to optionally emit a
dynamic switch to the main stack when they are on a suspender.

Comment 11

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - wasm: Remove save/restore of allocatable registers in some stubs. r?yury

The debug and request tier-up stubs save and restore all allocatable
registers. This is unnecessary and confusing as baseline does not have
any live registers when these stubs are called. Add an assertion
to that effect and drop this code.

Comment 12

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - wasm: Dynamically switch to main stack in import interp exit. r?yury

Use the infrastructure added for builtin thunks for the import
interp exit.

Comment 13

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - wasm: Dynamically switch to main stack in debug stub. r?yury

Comment 14

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - wasm: Dynamically switch to main stack in traps. r?yury

Comment 15

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - wasm: Use Wasm ABI for builtin thunks. r?yury

Builtin thunks are going to need more scratch registers for the
prologue/epilogue in order to do stack switches. Right now we
are using the sytem ABI for builtin thunks and this leaves us
with only 2 registers for the prologue and 1 register for the
epilogue. It is not feasible to expand this.

We could save/restore non-volatile registers, but this gets
really complicated when the stack is switching to figure out
which stack has the saved register on it.

An easier solution is to move builtin thunks back to using
the wasm ABI, which has no non-volatile registers and therefore
has a good amount of scratch registers that don't alias the
arg/result registers.

We still keep the code to call a builtin using the system ABI
and use that for builtins that don't have a thunk (such as
intgemm). A future commit will expand the number of builtins
which go without a thunk to reduce the overhead of the stack
switch.

A necessary change here is that we must now use saveLive/
restoreLive in some code that is around calling builtins
because we no longer have non-volatile registers.

Comment 16

[Ryan Hunt [:rhunt]]

Attachment: Bug 2002625 - Add documentation for ABI special registers. r?yury

This has needed better documentation for a long time.

Driveby fix for ARM64 to ArgRegMask to remove x8. Doesn't appear
to be used anywhere and so it should not be a functional change.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/54a523d23d99
https://hg.mozilla.org/integration/autoland/rev/d4639f7a6899
wasm: Merge SuspenderContext and wasm::Context. r=yury
https://github.com/mozilla-firefox/firefox/commit/bacf29978d81
https://hg.mozilla.org/integration/autoland/rev/ad9b3575badb
wasm: Move activeSuspender updating into wasm::Context. r=yury
https://github.com/mozilla-firefox/firefox/commit/2cf89694835f
https://hg.mozilla.org/integration/autoland/rev/c77ba16f9dca
wasm: Remove onSuspendableStack flag. r=yury
https://github.com/mozilla-firefox/firefox/commit/984eef92f3f7
https://hg.mozilla.org/integration/autoland/rev/84638b55a198
wasm: Move Win32 TIB updating into wasm::Context. r=yury
https://github.com/mozilla-firefox/firefox/commit/ad3f3d0b8bdf
https://hg.mozilla.org/integration/autoland/rev/888fe80a6574
wasm: Move arbitrary code assertions into wasm::Context. r=yury
https://github.com/mozilla-firefox/firefox/commit/990dfa57ae13
https://hg.mozilla.org/integration/autoland/rev/c9b0fc11148f
wasm: Store a set of all suspenders, not just suspended. r=yury
https://github.com/mozilla-firefox/firefox/commit/a5a3b6609dec
https://hg.mozilla.org/integration/autoland/rev/d8979e40924d
wasm: Drop suspendedBy field and use SuspenderState for this. r=yury
https://github.com/mozilla-firefox/firefox/commit/f8b8182ee696
https://hg.mozilla.org/integration/autoland/rev/e5969ca65b8c
wasm: Merge SuspenderObject and SuspenderObjectData. r=yury
https://github.com/mozilla-firefox/firefox/commit/1ab1eb6ce51b
https://hg.mozilla.org/integration/autoland/rev/9ad9ea0557aa
wasm: Simplify codegen after merging data and suspender. r=yury
https://github.com/mozilla-firefox/firefox/commit/527c3ea6fc52
https://hg.mozilla.org/integration/autoland/rev/fb1b7507031d
wasm: Use Wasm ABI for builtin thunks. r=yury
https://github.com/mozilla-firefox/firefox/commit/30e7d973d80c
https://hg.mozilla.org/integration/autoland/rev/9bcfab21ad81
wasm: Dynamically switch to main stack in builtin thunks. r=yury
https://github.com/mozilla-firefox/firefox/commit/67f6bf8eac64
https://hg.mozilla.org/integration/autoland/rev/be72032f10a2
Add documentation for ABI special registers. r=yury
https://github.com/mozilla-firefox/firefox/commit/09f04022c6f2
https://hg.mozilla.org/integration/autoland/rev/3c23f9a837d3
wasm: Remove save/restore of allocatable registers in some stubs. r=yury
https://github.com/mozilla-firefox/firefox/commit/a48c7813a3ef
https://hg.mozilla.org/integration/autoland/rev/96902a553d37
wasm: Dynamically switch to main stack in import interp exit. r=yury
https://github.com/mozilla-firefox/firefox/commit/9383bc69f749
https://hg.mozilla.org/integration/autoland/rev/8753cd32504b
wasm: Dynamically switch to main stack in debug stub. r=yury
https://github.com/mozilla-firefox/firefox/commit/fea5740ed0c1
https://hg.mozilla.org/integration/autoland/rev/5ca5a0667c11
wasm: Dynamically switch to main stack in traps. r=yury

Comment 18

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/d4639f7a6899
https://hg.mozilla.org/mozilla-central/rev/ad9b3575badb
https://hg.mozilla.org/mozilla-central/rev/c77ba16f9dca
https://hg.mozilla.org/mozilla-central/rev/84638b55a198
https://hg.mozilla.org/mozilla-central/rev/888fe80a6574
https://hg.mozilla.org/mozilla-central/rev/c9b0fc11148f
https://hg.mozilla.org/mozilla-central/rev/d8979e40924d
https://hg.mozilla.org/mozilla-central/rev/e5969ca65b8c
https://hg.mozilla.org/mozilla-central/rev/9ad9ea0557aa
https://hg.mozilla.org/mozilla-central/rev/fb1b7507031d
https://hg.mozilla.org/mozilla-central/rev/9bcfab21ad81
https://hg.mozilla.org/mozilla-central/rev/be72032f10a2
https://hg.mozilla.org/mozilla-central/rev/3c23f9a837d3
https://hg.mozilla.org/mozilla-central/rev/96902a553d37
https://hg.mozilla.org/mozilla-central/rev/8753cd32504b
https://hg.mozilla.org/mozilla-central/rev/5ca5a0667c11