fitzgen opened PR #13711 from fitzgen:wasmtime-wasm-block-params-should-be-variables to bytecodealliance:main:
This allows SSA construction to determine whether they need to actually become block params or not, and cuts down on the number of unnecessary block parameters we pessimistically introduce during Wasm-to-CLIF translation.
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
fitzgen requested cfallin for a review on PR #13711.
fitzgen requested wasmtime-compiler-reviewers for a review on PR #13711.
fitzgen requested wasmtime-core-reviewers for a review on PR #13711.
:thumbs_up: cfallin submitted PR review:
Thanks! This technically looks fine to me, and I agree it's nice to use our existing algorithm to avoid creating blockparams where they are actually trivial/unneeded.
I'm curious, however, since this does make things a little more complex (some blocks take vars, some don't), and leans more on an implicit SSA construction algorithm, which does have some cost, with the hypothesis that we'll get better code / faster compiles with fewer blockparams in the IR: have you measured the improvement? Do we see faster compile or faster runtimes out of this?
:speech_balloon: cfallin created PR review comment:
This distinction seems to me like it has potential to be error-prone or at least lead to subtle cases (e.g. there's the implicit invariant that the exit block is not also a Wasm target; we're creating a new bifurcation of kinds-of-blocks). Is there a reason we can't have the exit-block take its return value(s) as variables, too?
fitzgen commented on PR #13711:
I'm curious, however, since this does make things a little more complex (some blocks take vars, some don't), and leans more on an implicit SSA construction algorithm, which does have some cost, with the hypothesis that we'll get better code / faster compiles with fewer blockparams in the IR: have you measured the improvement? Do we see faster compile or faster runtimes out of this?
Just did a sightglass run on a subset of the suite recommended by an initial PCA analysis (before going through all the steps I still want to do before the Official analysis) and this PR doesn't have any statistically significant effect on either compilation or execution:
<details>
compilation :: cycles :: benchmarks/cm-online-stats/cm-online-stats.wasm No difference in performance. [61682 72049.07 117404] main.dylib [63866 80856.55 570407] vars.dylib compilation :: cycles :: benchmarks/splay/splay.wasm No difference in performance. [40436 50629.25 62620] main.dylib [39045 49528.01 59814] vars.dylib compilation :: cycles :: benchmarks/tinygo/tinygo-json.wasm No difference in performance. [1347717 1544747.12 1945877] main.dylib [1341698 1563298.14 1827803] vars.dylib compilation :: cycles :: benchmarks/hex-simd/benchmark.wasm No difference in performance. [188251 211734.34 259940] main.dylib [189635 214170.23 294342] vars.dylib execution :: cycles :: benchmarks/hex-simd/benchmark.wasm No difference in performance. [1163 1294.28 1621] main.dylib [1163 1280.12 1472] vars.dylib compilation :: cycles :: benchmarks/rust-protobuf/benchmark.wasm No difference in performance. [250100 273895.97 312874] main.dylib [247004 276475.62 514901] vars.dylib compilation :: cycles :: benchmarks/spidermonkey/spidermonkey-regex.wasm No difference in performance. [12437845 12799608.08 14087684] main.dylib [12399019 12868181.21 14477717] vars.dylib execution :: cycles :: benchmarks/rust-protobuf/benchmark.wasm No difference in performance. [26572 30433.83 35473] main.dylib [26738 30307.06 34453] vars.dylib compilation :: cycles :: benchmarks/blake3-scalar/benchmark.wasm No difference in performance. [196620 230621.09 321439] main.dylib [201370 229802.09 290981] vars.dylib execution :: cycles :: benchmarks/spidermonkey/spidermonkey-regex.wasm No difference in performance. [521266 536802.54 589523] main.dylib [518161 535075.71 630966] vars.dylib execution :: cycles :: benchmarks/tinygo/tinygo-json.wasm No difference in performance. [687485 711986.11 737841] main.dylib [692311 714065.42 743352] vars.dylib execution :: cycles :: benchmarks/splay/splay.wasm No difference in performance. [64684190 66353450.56 67354379] main.dylib [64723590 66454334.51 67421356] vars.dylib execution :: cycles :: benchmarks/cm-online-stats/cm-online-stats.wasm No difference in performance. [80048559 83716999.75 85692835] main.dylib [82024100 83823961.71 89048619] vars.dylib execution :: cycles :: benchmarks/blake3-scalar/benchmark.wasm No difference in performance. [1917 2113.29 2455] main.dylib [1915 2112.69 3030] vars.dylib</details>
fitzgen updated PR #13711.
:memo: fitzgen submitted PR review.
:speech_balloon: fitzgen created PR review comment:
I initially was trying to minimize changes by leaving the return block as-is, but switching it over as well means that basically all blocks are handled uniformly now (using variables instead of block params) except for two cases:
- The entry block, which has function params
- Exception blocks, which take the payload as params
However, both of these kinds of blocks are never targeted for branching directly by e.g.
br_ifin Wasm so they can never have Wasm stack operand params, so our handling of Wasm branch targets can still be uniform. I think this leaves us in a pretty nice place, but I'm curious what your opinion is too.
fitzgen added PR #13711 Translate Wasm block params into Variables instead of CLIF block params to the merge queue.
:check: fitzgen merged PR #13711.
fitzgen removed PR #13711 Translate Wasm block params into Variables instead of CLIF block params from the merge queue.
cfallin commented on PR #13711:
To close the loop on this: Nick and I talked offline and the Sightglass neutral results plus the cleanup unifying block types both look good to me, so :+1:
Last updated: Jul 29 2026 at 05:03 UTC