right now, we only insert safepoints at calls and interrupt traps:
it seems like we would really want to insert them at loop headers as well, to allow GC during long-lasting loops.
does anyone know if either (a) I'm overlooking some code that handles this for us, or (b) if we already have an issue on file for this (a cursory search failed to find any)? thanks!
FWIW, I did some digging last week on this (in the context of eventual reftype work) and also couldn't find anything for loops
filed https://github.com/bytecodealliance/wasmtime/issues/1815
but actually, now that I think about it, the loop header has to have some sort of check for whether the gc requested that the wasm synchronize and stop at a safepoint, which likely will involve some call, e.g it checks a flag hanging off the vmctx and (if set) calls into gc_synchronize
or something, and that call will get a safepoint. so maybe everything is fine here, and nothing needs to change...
I was curious how this works in wasm on Ion/x86, so I went digging a bit... here's the insertion of an interrupt-check op in the loop header: https://searchfox.org/mozilla-central/source/js/src/wasm/WasmIonCompile.cpp#2218 (MWasmInterruptCheck
IR node, lowered to LWasmInterruptCheck
) which eventually leads to this codegen: https://searchfox.org/mozilla-central/source/js/src/jit/CodeGenerator.cpp#13952
The interrupt check itself is a conditional branch (on some flag in the tlsdata) around a trap instruction: https://searchfox.org/mozilla-central/source/js/src/jit/MacroAssembler.cpp#3253
interestingly, it does seem to add a safepoint in the happy (non-interrupt) path, so I can only assume this is zero cost (i.e. doesn't spill anything): https://searchfox.org/mozilla-central/source/js/src/jit/CodeGenerator.cpp#13958
(actually, that safepoint is right at the return PC for the trap so semantically it's really part of the cold path; I suppose because only the return PC is available when the trap is handled)
in any case you're probably right that we'd be able to call some handler and we'd get the safepoint for free
So Cranelift-in-Spidermonkey generates an interrupt check as well, at the top of loop headers https://searchfox.org/mozilla-central/source/js/src/wasm/cranelift/src/wasm2clif.rs#1244, and this will end handling the interrupt the same way Spidermonkey does.
The translate_loop_header documentation explicitly mentions safepoints, but it seems we don't generate them at this point :thinking:
I just saw this link from @fitzgen (he/him) 's issue on github: https://github.com/bytecodealliance/wasmtime/blob/master/cranelift/codegen/src/regalloc/safepoint.rs#L54-L61
So yeah, since Spidermonkey generates this trapnz at loop headers, it will create a safepoint there. This sort of synchronization might be a bit fragile...
(And here's a new piece of information: it actually doesn't get added, because Spidermonkey generates trapif, not trap! Did i say this was fragile...)
Only resumable_trap get safepoints because otherwise nothing is considered live after the trap
Traps in Spidermonkey are resumable, so there might be something going on here...
Actually, the CLIR Trap and ReusableTrap would both get a safepoint created, only not instructions using the CondTrap format.
if you look at insert_and_encode_safepoint
, if there are no live references, then it skips the insertion of the safepoint, and for non-resumable traps (trap
) there are always no live references. I was playing around with this a bit yesterday trying to make emit_stackmaps
check if let Some(TrapCode::Interrupt) = pos.func.dfg[inst].trap_code()
instead of explicitly only checking for InstructionFormat::Trap
instructions
Last updated: Dec 23 2024 at 13:07 UTC