Wasmtime GitHub notifications bot (Apr 03 2026 at 16:24): Wasmtime GitHub notifications bot (Apr 03 2026 at 16:24):cfallin requested alexcrichton for a review on PR #12952.
Wasmtime GitHub notifications bot (Apr 03 2026 at 16:24):cfallin requested wasmtime-compiler-reviewers for a review on PR #12952.
Wasmtime GitHub notifications bot (Apr 03 2026 at 16:24):cfallin opened PR #12952 from cfallin:riscv64-trapif-fix to bytecodealliance:main:
The riscv64 backend has an instruction lowering for
trapifof anicmpthat emits a compare-and-branch instruction. When theicmphas an unsigned comparison mode and operates on narrower-than-64-bit values (e.g.I32s), the compare-and-branch requires that the values be zero-extended, since ordinarily registers have undefined bits above the contained type's width.Unfortunately the lowering didn't do this, so when the bits are actually nonzero, the comparison result is incorrect. For example, a 32-bit load (
lw) on riscv64 sign-extends the loaded value; if we make use of this, then we get an incorrect comparison result. The included test-case reproduces this issue.The fix is to use our existing helper to normalize the value (sign- or zero-extend as needed) for the comparison. (I had hoped to fold this logic into
gen_trapifto make it a little harder to skip, but at that level we're already dealing withXRegs rather thanValues so we've lost the type information and thus can't extend properly.)
Wasmtime GitHub notifications bot (Apr 03 2026 at 16:30):alexcrichton submitted PR review:
At some point we should probably re-align the various backends of how they handle things here. I feel like things are a bit mish-mash where idioms in one backend aren't found in other backends but we've still got sort of "one best-practices idiom" somewhere. For example I think the x64 backend has a helper that says "turn this
Valueinto a condition code thing" which deduplicates things liketrapzandselectandbrifand such to funnel all the conditionals into a single "handle all the*cmpinstructions here" which would centralize logic like this. Anyway, something tha can happen in the future, this is of course fine for now.
Wasmtime GitHub notifications bot (Apr 03 2026 at 16:30):alexcrichton has enabled auto merge for PR #12952.
Wasmtime GitHub notifications bot (Apr 03 2026 at 16:36):cfallin commented on PR #12952:
Yeah, definitely agreed -- we've learned enough "best practices" at this point to do so.
Wasmtime GitHub notifications bot (Apr 03 2026 at 16:47):alexcrichton added PR #12952 Cranelift: riscv64: fix zero-extension in trapif + icmp folding. to the merge queue
Wasmtime GitHub notifications bot (Apr 03 2026 at 17:10):alexcrichton merged PR #12952.
Wasmtime GitHub notifications bot (Apr 03 2026 at 17:10):alexcrichton removed PR #12952 Cranelift: riscv64: fix zero-extension in trapif + icmp folding. from the merge queue
Last updated: Apr 12 2026 at 23:10 UTC