Shuffle fields in VMRuntimeLimits
#9739
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Right now this structure has a pointer-sized field, two 64-bit integers, and then three pointer-sized fields. This structure's layout is nontrivial to calculate on 32-bit platforms as it needs to take the alignment of 64-bit integers into account which differs between ARM and x86 for example. To make this easier shuffle all the 64-bit integers are now first, which means we don't have to worry about alignment. I'll note that this particular ordering is still a bit brittle because we might need to shuffle things again if more fields are added. That being said any misalignment is caught during testing of the `wasmtime` crate so there's not much danger in adding more things, it'll just require updating a few more locations.
|
I'll note that the diff here looks scary but it's practically pretty small. The first commit is the actual change and the second commit is purely test expectation updates. This is because the offset of the stack-limit field has changed, and functions all have an explicit check of the stack limit. |
github-actions
bot
added
the
wasmtime:api
fitzgen
approved these changes
Dec 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Right now this structure has a pointer-sized field, two 64-bit integers,
and then three pointer-sized fields. This structure's layout is
nontrivial to calculate on 32-bit platforms as it needs to take the
alignment of 64-bit integers into account which differs between ARM and
x86 for example. To make this easier shuffle all the 64-bit integers are now
first, which means we don't have to worry about alignment.
I'll note that this particular ordering is still a bit brittle because
we might need to shuffle things again if more fields are added. That
being said any misalignment is caught during testing of the
wasmtimecrate so there's not much danger in adding more things, it'll just
require updating a few more locations.