Wasmtime GitHub notifications bot (Sep 01 2026 at 04:03): Wasmtime GitHub notifications bot (Sep 01 2026 at 04:03):cfallin opened PR #14254 from cfallin:u-mul-i-mul-we-all-mul-for-overflow to bytecodealliance:main:
This stacks on top of #14228, and adds the equivalent cases for multiplies.
This required some new cases in the
ProducesFlagsenum because, at least on aarch64, the lowerings are a little more complex than a single multiply instruction that sets flags (no such instruction exists on that ISA). The canonical idiom is instead to compare the product (full width) against the product extended-from-narrow-width; or, for 64-bit x 64-bit multiplies, use the separate high-half multiply instruction and compare against zero. This requires either two (8/16/32-bit case) or three (64-bit case) instructions to do the multiply and produce a flag for the overflow case.
Wasmtime GitHub notifications bot (Sep 01 2026 at 04:03):cfallin requested fitzgen for a review on PR #14254.
Wasmtime GitHub notifications bot (Sep 01 2026 at 04:03):cfallin requested wasmtime-compiler-reviewers for a review on PR #14254.
Wasmtime GitHub notifications bot (Sep 01 2026 at 04:03):cfallin requested wasmtime-core-reviewers for a review on PR #14254.
Wasmtime GitHub notifications bot (Sep 01 2026 at 04:03):cfallin commented on PR #14254:
(Stacked on #14228 until that PR merges)
Wasmtime GitHub notifications bot (Sep 01 2026 at 06:55):github-actions[bot] added the label cranelift on PR #14254.
Wasmtime GitHub notifications bot (Sep 01 2026 at 06:55):github-actions[bot] added the label cranelift:area:machinst on PR #14254.
Wasmtime GitHub notifications bot (Sep 01 2026 at 06:55):github-actions[bot] added the label cranelift:area:aarch64 on PR #14254.
Wasmtime GitHub notifications bot (Sep 01 2026 at 06:55):github-actions[bot] added the label cranelift:area:x64 on PR #14254.
Wasmtime GitHub notifications bot (Sep 01 2026 at 15:11):cfallin updated PR #14254.
Wasmtime GitHub notifications bot (Sep 01 2026 at 15:12):cfallin edited a comment on PR #14254:
(Stacked on #14228 until that PR merges)EDIT: rebased.
Wasmtime GitHub notifications bot (Sep 01 2026 at 16:25)::thumbs_up: alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Sep 01 2026 at 16:25)::speech_balloon: alexcrichton created PR review comment:
I feel like at some point we almost need
islefmtor similar, the consistency in these files are pretty wildly variable...(no actual need to change anything, just something I'm noticing)
Wasmtime GitHub notifications bot (Sep 01 2026 at 16:25)::speech_balloon: alexcrichton created PR review comment:
I'm not sure how applicable this is to the other cases, but could this be improved by basically discarding the
maddentirely here? I'd naively expect that if this lowering rule to be able to entirely avoid anything about opportunistic definitions it'd naturally fall out that this would useumulhto calculate the high bits and then later should the result of the multiplication be needed it'd see that the overflow flag was dead (in the common case consumed here) and then would only generate themadd
Wasmtime GitHub notifications bot (Sep 01 2026 at 16:25)::speech_balloon: alexcrichton created PR review comment:
I can't quite wrap my head around why this calculates overflow for both signed/unsigned, but I'm assuming you're basically following LLVM here
Wasmtime GitHub notifications bot (Sep 01 2026 at 16:25)::speech_balloon: alexcrichton created PR review comment:
Basically what I'm thinking is that the second result of a 64-bit multiply really is just
umulhas a standalone value that has nothing to do with the other result ofumul_overflow, so I'm wondering if that could be leveraged and sort of naturally fall out here
Wasmtime GitHub notifications bot (Sep 01 2026 at 16:47)::memo: cfallin submitted PR review.
Wasmtime GitHub notifications bot (Sep 01 2026 at 16:47)::speech_balloon: cfallin created PR review comment:
At least in terms of indenting, I've found Emacs' builtin lisp formatting to be a reasonable default (and I guess we could always run Emacs in batch-mode with a little elisp script to do it) but the higher-level question of when to add linebreaks is one that is pretty variable, yeah...
Wasmtime GitHub notifications bot (Sep 01 2026 at 16:49)::memo: cfallin submitted PR review.
Wasmtime GitHub notifications bot (Sep 01 2026 at 16:49)::speech_balloon: cfallin created PR review comment:
Ah! I see what you mean -- the two results are independently computable with no overlap at all so we might as well have two separate lowerings and let them both fire. Yeah, I'll play with that.
Wasmtime GitHub notifications bot (Sep 01 2026 at 18:03)::memo: cfallin submitted PR review.
Wasmtime GitHub notifications bot (Sep 01 2026 at 18:03)::speech_balloon: cfallin created PR review comment:
Ah, the smul cases below use
sxt{b,h}; this comment is just attached to a umul case (and note the equivalent comment below uses the signed versions).
Wasmtime GitHub notifications bot (Sep 01 2026 at 18:09):cfallin updated PR #14254.
Wasmtime GitHub notifications bot (Sep 01 2026 at 18:09)::memo: cfallin submitted PR review.
Wasmtime GitHub notifications bot (Sep 01 2026 at 18:09)::speech_balloon: cfallin created PR review comment:
OK, yep, that's much cleaner -- done, thanks!
Wasmtime GitHub notifications bot (Sep 01 2026 at 18:22):cfallin updated PR #14254.
Wasmtime GitHub notifications bot (Sep 01 2026 at 19:21):cfallin updated PR #14254.
Wasmtime GitHub notifications bot (Sep 01 2026 at 19:32):cfallin has enabled auto merge for PR #14254.
Wasmtime GitHub notifications bot (Sep 01 2026 at 19:40):cfallin added PR #14254 Cranelift: add umul_overflow / smul_overflow flags folding cases. to the merge queue
Wasmtime GitHub notifications bot (Sep 01 2026 at 20:19)::check: cfallin merged PR #14254.
Wasmtime GitHub notifications bot (Sep 01 2026 at 20:19):cfallin removed PR #14254 Cranelift: add umul_overflow / smul_overflow flags folding cases. from the merge queue
Last updated: Sep 20 2026 at 18:08 UTC