wasmtime / PR #14228 Cranelift: add opportunistic value d... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #14228 Cranelift: add opportunistic value d...

Wasmtime GitHub notifications bot (Aug 28 2026 at 20:34):

cfallin opened PR #14228 from cfallin:opportunistic-value-defs to bytecodealliance:main:

Add a new lowering primitive, an "opportunistic def", which allows a lowering that incidentally computes another value to register that value for possible later use. When the backward scan reaches the value's actual definition, if the use-count has not grown (no further uses appeared while scanning up), the definition can be skipped entirely and the value aliased to the opportunistically-computed regs.

Also add a value_used helper that reports whether a value still has uses, allowing skipping of some part of a lowering when not needed.

With these two features available in the lowering environment, this PR then adds new lowerings for brif-of-uadd_overflow, and bare uadd_overflow, on x86-64 and aarch64:

When uadd_overflow's overflow flag is used as a branch condition, the branch directly uses flags produced by the add instruction, skipping (slow and verbose) materialization of the bool overflow flag into a GPR.

If the sum is only used past the branch, then this add also produces that value; so only one add is ever emitted. Thus a add; SETcc; test; jnz sequence turns into (x64) add; jb / (aarch64) adds; b.hs.

If the sum is used below the uadd_overflow but above the branch, another add is also emitted. That's fine: adds are cheap; cheaper certainly than materializing flags with SETcc or CSET.

But to optimize that case further...

...when uadd_overflow is only used for its sum, and not its overflow flag, we now emit an add without a SETcc.

As a result of these lowerings, the case v1, v2 = uadd_overflow ...; brif v2, ... lowers to add; jb, and the case v1, v2 = uadd_overflow ...; (use v1); brif v2, ... lowers to add; (use sum); add; jb. The only remaining case where we use a slow SETcc is when the bool result is actually materialized and used as an integer value; or if the add and branch are pushed into separate blocks.

This resolves the same issue as #13919, following discussion in that PR and the Cranelift weekly meeting. In particular, (i) we do not build an ad-hoc separate scan (this mechanism works as part of the main lowering/instruction-selection scan); and (ii) we are resilient to instructions placed between the uadd_overflow and branch, which is likely to happen due to egraph demand-based elaboration, and which foils a simpler peephole-based approach.

Wasmtime GitHub notifications bot (Aug 28 2026 at 20:34):

cfallin requested alexcrichton for a review on PR #14228.

Wasmtime GitHub notifications bot (Aug 28 2026 at 20:34):

cfallin requested wasmtime-compiler-reviewers for a review on PR #14228.

Wasmtime GitHub notifications bot (Aug 28 2026 at 20:35):

cfallin commented on PR #14228:

(This has been on my TODO list for a long time; sorry about the delay, @playX18, but hopefully this addresses your use-case!)

Wasmtime GitHub notifications bot (Aug 28 2026 at 21:07):

:memo: alexcrichton submitted PR review:

Seems reasonable to me! Question on this: would it be possible to somehow determine, before fusing this into a branch, if the opportunistic def is going to be thrown away and re-calculated? For something like an addition it seems probably fine to always duplicate that, but for something like a multiplication it might be better to never duplicate that given its latency. (maybe? unsure). Basically it seems to me like a reasonable heuristic here would be to only fuse into branches where the opportunistic def actually works as the def, and in all other cases fall back to materializing the flag and then testing it later.

Although now that I actually write this down what I'm going for is to put the onus on CLIF producers to make sure the flag-and-branch are close together such that the opportunistic def always matches. In some sense that's no different from this PR as-is where it's still on them to do that to avoid the double-translate if it matters... Anyway, still curious on the question at least as a data point

Wasmtime GitHub notifications bot (Aug 28 2026 at 21:07):

:speech_balloon: alexcrichton created PR review comment:

Would it be possible to fold this into is_nonzero_cmp instead of having a rule specifically for brif? That'd then additionally handle the condition flowing intp a trapz, trapnz, and select all at the same time. If this does work, could this be done for x64 too?

Wasmtime GitHub notifications bot (Aug 28 2026 at 21:07):

:speech_balloon: alexcrichton created PR review comment:

Is this rule needed for the codegen tests that were added, or was this opportunistically added mostly to prove this out?

Additionally, could this skip using adds and just use add instead? If so I think the ConsumesFlagsNop infrastructure can be removed.

Wasmtime GitHub notifications bot (Aug 28 2026 at 21:07):

:speech_balloon: alexcrichton created PR review comment:

Additionally, since we might want to use this for a number of other things, could a new helper like produces_flags_with_opportunistic_def be added? That'd internally then use produces_flags_get_reg, call opportunistic_def, and then return a ProducesFlags created with produces_flags_ignore. Basically factoring out the boilerplate-y parts of this if possible

Wasmtime GitHub notifications bot (Aug 28 2026 at 21:45):

cfallin requested pchickey for a review on PR #14228.

Wasmtime GitHub notifications bot (Aug 28 2026 at 21:45):

cfallin requested wasmtime-core-reviewers for a review on PR #14228.

Wasmtime GitHub notifications bot (Aug 28 2026 at 21:45):

cfallin updated PR #14228.

Wasmtime GitHub notifications bot (Aug 28 2026 at 21:45):

:memo: cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 28 2026 at 21:45):

:speech_balloon: cfallin created PR review comment:

Yes, this is what handles the "intermediate user" case (and removes the unused SETcc). But yeah great point, I dunno why I kept that bit (copy+paste I guess); this is really just a plain old add. Updated!

Wasmtime GitHub notifications bot (Aug 28 2026 at 21:48):

cfallin commented on PR #14228:

Seems reasonable to me! Question on this: would it be possible to somehow determine, before fusing this into a branch, if the opportunistic def is going to be thrown away and re-calculated? For something like an addition it seems probably fine to always duplicate that, but for something like a multiplication it might be better to never duplicate that given its latency. (maybe? unsure). Basically it seems to me like a reasonable heuristic here would be to only fuse into branches where the opportunistic def actually works as the def, and in all other cases fall back to materializing the flag and then testing it later.

Although now that I actually write this down what I'm going for is to put the onus on CLIF producers to make sure the flag-and-branch are close together such that the opportunistic def always matches. In some sense that's no different from this PR as-is where it's still on them to do that to avoid the double-translate if it matters... Anyway, still curious on the question at least as a data point

Unfortunately we can't really know "the future" during the backward lowering scan (or, well, we could, but that would be a separate scan). The conclusion I came to is that it's way simpler to build this opportunistic mechanism and just let the lowering fire twice if there is actually a user in the block in between the add and branch.

I also address this with "If the sum is used below the uadd_overflow but above the branch, another add is also emitted. That's fine: adds are cheap; cheaper certainly than materializing flags with SETcc or CSET.", I think: what we get here is actually still cheaper than what you're asking for (strictly one fewer instruction -- add; add; branch rather than add; setcc; test; branch; and the setcc is slow).

Wasmtime GitHub notifications bot (Aug 28 2026 at 22:18):

:memo: cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 28 2026 at 22:18):

:speech_balloon: cfallin created PR review comment:

I played with this a bit, but IMHO it's not really that much simpler to fold this into ProducesFlags. It requires pulling apart all the pieces from the original inst anyway and stuffing them into the ProducesFlags (the other result value, the emitted result from the adds) and requires thinking about the emission combinations with various ConsumesFlags. Arguably more brittle, and distributes the logic over a larger area. In other words I don't think it actually avoids any boilerplate, it just explodes it into shards of boiler iron embedded in the walls...

Wasmtime GitHub notifications bot (Aug 28 2026 at 22:19):

:memo: cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 28 2026 at 22:19):

:speech_balloon: cfallin created PR review comment:

(Happy to tackle this or see someone else tackle this in a followup of course if there are more ideas here -- but maybe good to get the building blocks in first regardless?)

Wasmtime GitHub notifications bot (Aug 28 2026 at 23:44):

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

Wasmtime GitHub notifications bot (Aug 28 2026 at 23:44):

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

Wasmtime GitHub notifications bot (Aug 28 2026 at 23:44):

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

Wasmtime GitHub notifications bot (Aug 28 2026 at 23:44):

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

Last updated: Aug 30 2026 at 09:07 UTC