Remove the initializer from a global's type information#6349
Conversation
This commit removes the `Global::initializer` field to instead only store type information about the global rather than its initialization state. Instead now the initializer is stored separately from the type of the global, and only for defined globals. This removes the need in a few locations to synthesize dummy initializers.
Subscribe to Label Actioncc @peterhuene DetailsThis issue or pull request has been labeled: "cranelift", "cranelift:wasm", "wasmtime:api"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
jameysharp
left a comment
There was a problem hiding this comment.
This is great. There's one leftover bit I think you meant to delete and one thing maybe you can make more clear, but overall this makes perfect sense.
| ptr::write(to, VMGlobalDefinition::new()); | ||
|
|
||
| match global.initializer { | ||
| match module.global_initializers[def_index] { |
There was a problem hiding this comment.
The connection between indexes of global_initializers and defined_global_index seems a little subtle to me, but maybe that's just because I'm not that used to these idioms. I think defined_global_index should just be subtracting off num_imported_globals, right? Could you use Iterator::zip to join the initializers to their definitions?
Feel free to leave this as is, but if you see an easy way to make this more obviously correct that'd be nice.
There was a problem hiding this comment.
Ah yes indeed this can be simplified! I've switched it to iterate over module.global_initializers which is more direct and clearer.
This commit removes the
Global::initializerfield to instead only store type information about the global rather than its initialization state. Instead now the initializer is stored separately from the type of the global, and only for defined globals. This removes the need in a few locations to synthesize dummy initializers.