bytecodealliance / bytecodealliance/wasmtime

Split "known entity import" and "everyone knows this entity's import" into two different dimensions

Open
#14,164 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Rust
Stars
18.6k
Forks
1.8k
Avg merge
1d 19h
Merged PRs (30d)
121

Description

> Reading over this again I personally find this pretty confusing. I understand why this is the way it is, but in consider this I could imaging an alternative design where a module statically knows it imports a particular global and then has separate information on whether it's appropriate to use a statically known alias region for that global. Basically I could imagine a design where one axis of imports is "is it always this thing" and then another axis is "is that thing always known to all other modules as well".
>
>One example optimization with this is that if we know an import of a non-mutable global is always a particular global we can just inline the value everywhere. That's got nothing to do with alias regions, however, and by tying alias regions to this known-imports set we're unable to optimize some situations.
>
> I don't mean to place more work on this PR, but I want to explain my confusion with the phrasing/naming here. The "XXX" here seems to indicate that this is a strong requirement that these sets must always be different from the functions set, but I don't feel that fully describes the situation.
>
> Given all that, two questions:
>
> * Could this actually be relaxed where "known imports" are unconditionally "this module is only ever instantiated with this thing"? In such a situation I'd imagine that when deducing the alias region for an imported global, for example, it'd see the known import and then lookup in that defining `ModuleTranslation` if the global is in the `globals_known_to_importers` set. I'm not sure if we've got all the sibling `ModuleTranslation`s at compile time available to make this deduction.
> * Failing that, could the comment here be expanded with some of the commentary I have here? Basically that this is an open issue we could consider fixing in the future and the "XXX" here isn't a hard requirement, just an artifact of the current implementation.

_Originally posted by @alexcrichton in https://github.com/bytecodealliance/wasmtime/pull/14115#discussion_r3806460442_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.