wabt
wabt copied to clipboard
wasm-interp: Handle Refs
This fills in some TODOs, may or may not be relevant for EHv4 but will definitely be relevant for GC later on.
We tried our best to make this efficient, hence the interpreter-only opcodes.
@sbc100 (rebased, ptal)
oh yeah, we should probably bring it up: an alternative to the funny stuff with locals and refs_ would be to mark ref locals at function start, which would be decidedly faster if slightly more annoying to set up. but then again, we weren't really going for maximum performance with this.
we don't know how complete the interpreter GC (not to be confused with the GC proposal and its implementation) is (how do you even test something like that), but this "should" mark refs correctly now. we think. we're pretty sure, but we wouldn't mind a second opinion.
as we were saying, we're not sure if there's a (good) way to test this?
as far as we can tell, Store::Collect is never called while the interpreter is running, so it kinda just... leaks?
So are you proposing landing this without tests? Or is this still a draft PR with more changes coming maybe?
we suppose we can try making the GC run regularly and see if it blows up... (but actually enabling the GC to run on its own should be a separate change)
running GC every instruction does indeed blow up.
we can't currently add tests (actually this doesn't need new tests as much as it needs a way to run the existing tests with actual collection enabled), blocked on fixing a nasty issue with spectest-interp, but these changes do fix a bunch of assertion failures when running store_.Collect() for every instruction. we would indeed like to land this without tests (for now).
any chance of getting this merged? @sbc100
I suppose I'm ok with landing this as an internal refactor. Hopefully any followups can have tests?