wasmtime / PR #13354 cranelift: Turn `AliasRegion` into a... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #13354 cranelift: Turn `AliasRegion` into a...

Wasmtime GitHub notifications bot (May 13 2026 at 18:34):

fitzgen opened PR #13354 from fitzgen:alias-region-entity to bytecodealliance:main:

Generalize from
enum AliasRegion {
    Heap,
    Table,
    Vmctx,
}
into a proper entity, allowing users to define arbitrary numbers of custom alias
regions.

MemFlagsData grows a region: PackedOption<AliasRegion> field instead of
bitpacking the alias region with the rest of its flags.

The CLIF text format is also updated to support alias region declarations in the
function header, e.g. region0 = 0 "heap", with memflags referencing regions by
entity name instead of the old hardcoded keywords.

Depends on https://github.com/bytecodealliance/wasmtime/pull/13353

Wasmtime GitHub notifications bot (May 13 2026 at 18:34):

fitzgen requested wasmtime-compiler-reviewers for a review on PR #13354.

Wasmtime GitHub notifications bot (May 13 2026 at 18:34):

fitzgen requested cfallin for a review on PR #13354.

Wasmtime GitHub notifications bot (May 13 2026 at 18:34):

fitzgen requested wasmtime-compiler-s390x-reviewers for a review on PR #13354.

Wasmtime GitHub notifications bot (May 13 2026 at 18:34):

fitzgen requested wasmtime-core-reviewers for a review on PR #13354.

Wasmtime GitHub notifications bot (May 13 2026 at 18:46):

fitzgen updated PR #13354.

Wasmtime GitHub notifications bot (May 13 2026 at 20:18):

github-actions[bot] added the label cranelift:area:aarch64 on PR #13354.

Wasmtime GitHub notifications bot (May 13 2026 at 20:18):

github-actions[bot] added the label cranelift on PR #13354.

Wasmtime GitHub notifications bot (May 13 2026 at 20:18):

github-actions[bot] added the label cranelift:meta on PR #13354.

Wasmtime GitHub notifications bot (May 13 2026 at 20:18):

github-actions[bot] added the label isle on PR #13354.

Wasmtime GitHub notifications bot (May 13 2026 at 20:18):

github-actions[bot] commented on PR #13354:

Subscribe to Label Action

cc @cfallin, @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:meta", "isle"

Thus the following users have been cc'd because of the following labels:

cfallin: isle

fitzgen: isle

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (May 13 2026 at 21:55):

:memo: cfallin submitted PR review:

Thanks! Some thoughts below but overall shape looks good. Happy this worked out w.r.t. compile-time overheads!

Wasmtime GitHub notifications bot (May 13 2026 at 21:55):

:speech_balloon: cfallin created PR review comment:

we can just say "unused" here I guess -- no need to embed the history of the bits in the comment?

Wasmtime GitHub notifications bot (May 13 2026 at 21:55):

:speech_balloon: cfallin created PR review comment:

Duplicated comment (maybe a merge or rebase issue?)

Wasmtime GitHub notifications bot (May 13 2026 at 21:55):

:speech_balloon: cfallin created PR review comment:

PackedOption maybe so None can fit in the reserved_value niche? I know this won't be as memory-critical today because it's dedup'd but if we end up with a lot of these it might still matter...

Wasmtime GitHub notifications bot (May 13 2026 at 21:55):

:speech_balloon: cfallin created PR review comment:

As long as we're manually implementing Hash we could do so for PartialEq/Eq as well for efficiency, since we can skip dedupe_map for that too (it's purely derived data), right?

Wasmtime GitHub notifications bot (May 13 2026 at 21:55):

:speech_balloon: cfallin created PR review comment:

Should this be the point in the API that's fallible, so we can propagate the u16-index-space allocation failure upward (e.g. to wasmtime-cranelift when it uses these for a user-controlled number of regions)?

Wasmtime GitHub notifications bot (May 13 2026 at 21:55):

:speech_balloon: cfallin created PR review comment:

Since the last-fence becomes the fallback, could we clear regions here instead?

Wasmtime GitHub notifications bot (May 13 2026 at 21:55):

:speech_balloon: cfallin created PR review comment:

This seems like a fairly awkward and inefficient bit of plumbing to get the MemFlagsSet down to the point of emission -- maybe we could pass a borrow into MachInst::emit instead?

Also, separately, it's a somewhat conspicuous layering violation: we have an IR type all the way down in the VCode. I'm not completely categorically opposed to that in all cases, but I wonder if we could lower MemFlagData to a "core" memflags actually semantically meaningful at the VCode layer (so, no alias regions any more, just trap info and the like) and use that in the backends?

Wasmtime GitHub notifications bot (May 13 2026 at 21:55):

:speech_balloon: cfallin created PR review comment:

here's another example where we might benefit from having a VCode-specific "core memflags": the printed form here is relatively useless (refers to an index of an entity that we don't print anywhere else in the disassembly) whereas ideally we'd print the same as before, after lowering.

Wasmtime GitHub notifications bot (May 20 2026 at 23:40):

fitzgen updated PR #13354.

Wasmtime GitHub notifications bot (May 20 2026 at 23:41):

fitzgen commented on PR #13354:

@cfallin okay I fixed the rebase, cleaned up the region clearing, and introduced MachMemFlags. Think this is ready for re-review!

Wasmtime GitHub notifications bot (May 21 2026 at 04:02):

github-actions[bot] added the label cranelift:area:x64 on PR #13354.

Wasmtime GitHub notifications bot (May 22 2026 at 17:03):

:thumbs_up: cfallin submitted PR review:

Generally looks good -- just some nits below. Thanks!

Wasmtime GitHub notifications bot (May 22 2026 at 17:03):

:speech_balloon: cfallin created PR review comment:

Maybe we could move the definition here instead of defining it in ir and aliasing it?

Wasmtime GitHub notifications bot (May 22 2026 at 17:03):

:speech_balloon: cfallin created PR review comment:

Eek -- unsafe-transmute: can we add a const constructor if we need one instead?

Wasmtime GitHub notifications bot (May 22 2026 at 17:03):

:speech_balloon: cfallin created PR review comment:

comment nit: same as MemFlagsData's previous representation, but we've now moved it solely into this type (and composed the outer struct around it), no? It might be good to also move the bitpacking comment here (to the field) since we discarded it above.

Wasmtime GitHub notifications bot (May 22 2026 at 19:53):

fitzgen updated PR #13354.

Wasmtime GitHub notifications bot (May 22 2026 at 19:55):

:memo: fitzgen submitted PR review.

Wasmtime GitHub notifications bot (May 22 2026 at 19:55):

:speech_balloon: fitzgen created PR review comment:

I had thought this would require invasive changes to the ReservedValue trait, but realized I could avoid that by adding a raw constructor to PackedOption that just takes the inner value and its Some if the raw inner value is not the reserved value, and None otherwise.