Skip to content

Limit alias regions in Wasm-to-CLIF translation to 256 - #14253

Merged
fitzgen merged 3 commits into
bytecodealliance:mainfrom
fitzgen:limit-number-of-alias-regions
Sep 4, 2026
Merged

Limit alias regions in Wasm-to-CLIF translation to 256#14253
fitzgen merged 3 commits into
bytecodealliance:mainfrom
fitzgen:limit-number-of-alias-regions

Conversation

@fitzgen

@fitzgen fitzgen commented Sep 1, 2026

Copy link
Copy Markdown
Member

Instead of bitpacking the AliasRegionKey into a u32, hash it and then xor-fold the hash down to one byte.

Fixes #14221

Depends on #14230

@fitzgen
fitzgen requested review from a team as code owners September 1, 2026 00:29
@fitzgen
fitzgen requested review from cfallin and removed request for a team September 1, 2026 00:29
@github-actions github-actions Bot added the cranelift Issues related to the Cranelift code generator label Sep 1, 2026
@gfx

gfx commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

This fixes the problem I reported in #14210.

I used the generator script from that issue and ran wasmtime compile -o /dev/null. Release build, best of 3, macOS / aarch64 only.

N base ffb04089ea this PR
250 0.14 s 0.08 s
500 0.97 s 0.19 s
1000 6.86 s 0.40 s
2000 51.48 s 0.87 s
4000 -- 1.79 s
8000 -- 4.15 s

Time roughly doubles when N doubles, out to N=8000. The blowup is gone and my
module builds again. Thank you.

FYI, the RPO worklist from #14211 is still 3-5x faster on top of this PR
(0.16 s at N=2000, 1.39 s at N=8000), but the gap narrows as N grows, so it is
only a constant factor now. However, I am closing #14211. It conflicts, and this PR fixes what I actually
hit.

One request: if this misses the 49.0.0 branch on the 5th, could it be
backported to release-48.0.0? The slowdown is a serious regression in 48.0.0, and
48.0.x is the current release. I know patch releases are meant for
security and correctness fixes and that backports are volunteer work, so this
is only a request.

@cfallin cfallin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review on the last commit only (the lattice commit still has work in progress as I understand it from our discussion yesterday, re: scoped hashmap and such).

My main concern with the machinery here is the description-string complexity. Seeing the "reverse map" that keeps original identities around, and especially the string-joining machinery, inside Cranelift itself, both (i) raises serious efficiency concerns for me (we may be at a net speedup by preventing alias-analysis state blowup, but we are still introducing a lot of string munging to produce strings that most compilations will never need), and (ii) seems more complex than it needs to be.

I wonder if we could simply drop the descriptions, and instead emit log::trace lines for mapping of alias-region keys to Cranelift region numbers, and let that suffice for debugging? It seems that all the same information would be there, but we wouldn't be eagerly computing then throwing it away on every compilation...

Comment thread crates/cranelift/src/alias_region.rs Outdated
Comment thread crates/cranelift/src/alias_region.rs Outdated
@fitzgen
fitzgen force-pushed the limit-number-of-alias-regions branch 2 times, most recently from b6f8585 to 50502ed Compare September 4, 2026 15:51
@fitzgen

fitzgen commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

This is now rebased on main, now that #14230 landed. Should be ready to merge once it has your approval, @cfallin.

@cfallin cfallin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@cfallin

cfallin commented Sep 4, 2026

Copy link
Copy Markdown
Member

(There's a clippy failure on a u128 as u64 truncation; I guess it needs a pragma of some sort to placate it)

Instead of bitpacking the `AliasRegionKey` into a `u32`, hash it and then
`xor`-fold the hash down to one byte.

Fixes bytecodealliance#14221
@fitzgen
fitzgen force-pushed the limit-number-of-alias-regions branch from 4deeb9b to 5e00554 Compare September 4, 2026 18:32
@fitzgen
fitzgen enabled auto-merge September 4, 2026 18:38
@fitzgen
fitzgen added this pull request to the merge queue Sep 4, 2026
Merged via the queue into bytecodealliance:main with commit cb4c215 Sep 4, 2026
54 checks passed
@fitzgen
fitzgen deleted the limit-number-of-alias-regions branch September 4, 2026 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift Issues related to the Cranelift code generator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Excessive compile time in alias analysis for 500 live structs on the stack

3 participants