cfallin requested fitzgen for a review on PR #14272.
cfallin requested wasmtime-compiler-reviewers for a review on PR #14272.
cfallin opened PR #14272 from cfallin:fuzzbug-fix to bytecodealliance:main:
In #14271 we got a fuzzbug that had a
uadd_overflowwith (i) a sinkable load as one argument, (ii) multiple uses of the flag output, so its flag result would have to be materialized anyway.This results in the lowering happening twice: once to feed the flags into the branch, and once to actually materialize the bool value. That's fine: the whole point of the new mechanism is that the common case is to use the overflow as a branch input only, not as a materialized bool, so a double lowering here is harmless (two extra cycles).
The issue arises because of a bit of logic I had forgotten we added several years ago in #9510 that declares all multi-def instructions as "value roots" that will only be lowered once. This allows loads to sink into such instructions always (if only used once by that instruction of course), but in turn requires a promise that we will do what we say on the tin: we will only ever lower any multi-def instruction once.
I believe that this eliminates any practical way of optimizing overflow-flag insts, or doing fusion of bool flags into conditional branches at all, because these inherently require lowering at use sites (because of the way that we don't regalloc flags).
It is also a somewhat dangerous (IMHO, now with perspective) exception to our otherwise principled "multiplicity" analysis: we otherwise assume (i) that any given instruction is only lowered once, unless (ii) truly used multiple times in the DFG. That is what allows us to reason about code motion of loads in a princpled way (because we can't duplicate a load). That principle is pretty simple; declaring some instructions "roots" and allowing them to "kill" multiplicity adds this footgun that will strike whenever we forget the exception and write a lowering rule like the overflow cases.
Fortunately it seems we don't actually get any test failures when removing that feature (and the test from #9510 is still in-tree?), so it's not required anymore; so this PR removes the feature. The attached test will panic without the fix.
Fixes #14271.
(Some more philosophical thought: our pre-pass that computes multiplicity is itself a choice, but the alternative requires us to give up single-pass lowering altogether and forces code motion into an iterative/fixpoint kind of framework. Consider: we have multiple uses of a given load; when we see the first use, how do we know whether it's the only use (and we can sink it into here) or there will be another? The multiplicity analysis is what answers that ahead of time, and despite its main limitation (it cannot be updated live), it seems to work well overall if we stick to the framework.)
<!--
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 review the Bytecode Alliance's AI tool usage policy at
https://github.com/bytecodealliance/governance/blob/main/AI_TOOL_POLICY.mdPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
cfallin updated PR #14272.
github-actions[bot] added the label cranelift on PR #14272.
github-actions[bot] added the label cranelift:area:machinst on PR #14272.
:thumbs_up: fitzgen submitted PR review.
fitzgen added PR #14272 Cranelift: fix uadd_overflow + load sinking. to the merge queue
:check: fitzgen merged PR #14272.
fitzgen removed PR #14272 Cranelift: fix uadd_overflow + load sinking. from the merge queue
Last updated: Sep 20 2026 at 18:08 UTC