wasmtime-cranelift: Reorder table GC write barrier #8137
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have some flexibility in what order we do steps in the GC write barrier for
table.set
on anexternref
table. We must:But that leaves several valid orders for these steps.
This commit moves the load and store as early as possible, so they're right after bounds-checking.
I believe this should have no effect on the correctness of this write barrier. It enables an upcoming change I'm working on to let table bounds-checks trap in the memory access instruction, rather than during address computation. The invariants here are tricky though so I wanted to get this change reviewed in isolation first.
For my purposes, I only need to move the load earlier; the store could stay where it was, right after incrementing the new refcount. However, moving both memory accesses shortens the range where the table entry address needs to be in a register, without lengthening the live range of any other value. Moving the load does lengthen the live range of the old ref, but as long as we move the store with it, register pressure should be unchanged.
Moving the store before the corresponding refcount increment should be safe for two reasons. One is that there's no opportunity for GC to happen in between. The other is that the new value should already have a non-zero refcount.