Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linking tags #215

Merged
merged 8 commits into from
Aug 23, 2024
Merged

Linking tags #215

merged 8 commits into from
Aug 23, 2024

Conversation

dhil
Copy link
Member

@dhil dhil commented Aug 20, 2024

This patch implements support for importing and exporting tags in the virtual machine. As a consequence tags are now unique to their origin instance, meaning that two different instantiations of the same module yields distinct tags.

The runtime representation of a tag has changed from an u32 to a usize (or native pointer). The identity of a tag is given by its address in the vmcontext.

Resolves #25.

@dhil dhil requested a review from frank-emrich August 20, 2024 12:36
@dhil
Copy link
Member Author

dhil commented Aug 21, 2024

I haven't implemented the proper runtime representation of tags in the baseline implementation yet. I am planning to defer that to a separate patch.

This patch implements support for importing and exporting tags in the
virtual machine. As a consequence tags are now unique to their origin
instance, meaning that two different instantiations of the same module
yields distinct tags.

The runtime representation of a tag has changed from an `u32` to a
`usize` (or native pointer). The identity of a tag is given by its
address in the vmcontext.

Resolves wasmfx#25.
Copy link

@frank-emrich frank-emrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This generally seems to be in good shape, mostly some questions about tag-related types.

crates/cranelift/src/wasmfx/optimized.rs Outdated Show resolved Hide resolved
crates/cranelift/src/wasmfx/optimized.rs Outdated Show resolved Hide resolved
crates/wasmtime/src/runtime/vm/vmcontext.rs Show resolved Hide resolved
crates/wasmtime/src/runtime/vm/vmcontext.rs Outdated Show resolved Hide resolved
crates/types/src/lib.rs Show resolved Hide resolved
cranelift/wasm/src/sections_translator.rs Show resolved Hide resolved
crates/wasmtime/src/runtime/types.rs Show resolved Hide resolved
Copy link

@frank-emrich frank-emrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good now.

@dhil dhil merged commit d66ad85 into wasmfx:main Aug 23, 2024
37 checks passed
@dhil dhil deleted the wasmfx-linking-tags branch August 23, 2024 13:10
@dhil dhil mentioned this pull request Oct 4, 2024
dhil added a commit that referenced this pull request Oct 4, 2024
This patch updates the README to reflect the recent changes to the CLI.

I think the following bullet point in the limitations subsection is
outdated:
> - Only a single module can be executed. In particular, providing
additional
> modules using the `--preload` option of `wasmtime` can lead to
unexpected
>  behavior.

I think this should work after the proper tags patch #215. Though, I
haven't tested it yet, so I will leave it be for now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clash of the tags
2 participants