Limit alias regions in Wasm-to-CLIF translation to 256 - #14253
Conversation
|
This fixes the problem I reported in #14210. I used the generator script from that issue and ran
Time roughly doubles when N doubles, out to N=8000. The blowup is gone and my FYI, the RPO worklist from #14211 is still 3-5x faster on top of this PR One request: if this misses the 49.0.0 branch on the 5th, could it be |
cfallin
left a comment
There was a problem hiding this comment.
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...
b6f8585 to
50502ed
Compare
|
(There's a clippy failure on a |
Instead of bitpacking the `AliasRegionKey` into a `u32`, hash it and then `xor`-fold the hash down to one byte. Fixes bytecodealliance#14221
4deeb9b to
5e00554
Compare
Instead of bitpacking the
AliasRegionKeyinto au32, hash it and thenxor-fold the hash down to one byte.Fixes #14221
Depends on #14230