alexcrichton opened PR #6349 from alexcrichton:less-global-init
to bytecodealliance:main
:
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.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
alexcrichton requested jameysharp for a review on PR #6349.
alexcrichton requested wasmtime-compiler-reviewers for a review on PR #6349.
alexcrichton requested wasmtime-core-reviewers for a review on PR #6349.
jameysharp submitted PR review:
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.
jameysharp submitted PR review:
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.
jameysharp created PR review comment:
I assume this should be deleted instead of commented out.
jameysharp created PR review comment:
The connection between indexes of
global_initializers
anddefined_global_index
seems a little subtle to me, but maybe that's just because I'm not that used to these idioms. I thinkdefined_global_index
should just be subtracting offnum_imported_globals
, right? Could you useIterator::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.
alexcrichton updated PR #6349.
alexcrichton created PR review comment:
Ah yes indeed this can be simplified! I've switched it to iterate over
module.global_initializers
which is more direct and clearer.
alexcrichton has enabled auto merge for PR #6349.
jameysharp created PR review comment:
Nice! Looks great.
alexcrichton merged PR #6349.
Last updated: Dec 23 2024 at 12:05 UTC