Wasmtime GitHub notifications bot (May 20 2025 at 19:34): Wasmtime GitHub notifications bot (May 20 2025 at 19:34):alexcrichton requested cfallin for a review on PR #10817.
Wasmtime GitHub notifications bot (May 20 2025 at 19:34):alexcrichton opened PR #10817 from alexcrichton:fix-imul-fuzz to bytecodealliance:main:
This commit fixes a minor regression from #10782 found via fuzzing. The regression is 16-bit immediates were forced to fit from an
i32value into au16for 16-bit multiplication. This meant though that negative numbers failed this conversion which meant that ISLE would panic due to the value not being matched. This fixes the logic to first fit the i32 into an i16 and then cast that to a u16 where the first phase should hit all the constants that are possible in Cranelift.<!--
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
-->
Wasmtime GitHub notifications bot (May 20 2025 at 19:34):alexcrichton requested wasmtime-compiler-reviewers for a review on PR #10817.
Wasmtime GitHub notifications bot (May 20 2025 at 19:34):alexcrichton requested dicej for a review on PR #10817.
Wasmtime GitHub notifications bot (May 20 2025 at 19:34):alexcrichton requested wasmtime-core-reviewers for a review on PR #10817.
Wasmtime GitHub notifications bot (May 20 2025 at 19:36):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (May 20 2025 at 19:36):alexcrichton created PR review comment:
I'll admit this is testing the limits of my CLIF knowledge. This will fail when the
i32value fits in au16, but not ani16, meaning that we'll still panic during instruction selection from that. I don't know whether that's possible in CLIF though, or ificonstvalues are always sign-extended from their type.
Wasmtime GitHub notifications bot (May 20 2025 at 19:36):cfallin submitted PR review.
Wasmtime GitHub notifications bot (May 20 2025 at 19:36):cfallin has enabled auto merge for PR #10817.
Wasmtime GitHub notifications bot (May 20 2025 at 19:52):cfallin created PR review comment:
Oh, missed that, sorry -- unfortunately we've standardized on zero extension instead (it keeps the majority of cases simpler). Fortunately if this extractor returns
Nonethen the rule just doesn't match -- no runtime panic unless there's not another fallback? Let's (i) add a test case with a value in that range (say 40000) and (ii) verify there is...
Wasmtime GitHub notifications bot (May 20 2025 at 19:52):cfallin submitted PR review.
Wasmtime GitHub notifications bot (May 20 2025 at 21:17):alexcrichton updated PR #10817.
Wasmtime GitHub notifications bot (May 20 2025 at 21:19):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (May 20 2025 at 21:19):alexcrichton created PR review comment:
Ok I've added a few CLIF tests for this and it looks like things are actually working as intended. Looks like when accessing the constant in ISLE it gets sign-extended based on the type of the constant itself, which is why this ended up working out. Yay!
Wasmtime GitHub notifications bot (May 20 2025 at 21:19):alexcrichton has enabled auto merge for PR #10817.
Wasmtime GitHub notifications bot (May 20 2025 at 21:52):alexcrichton merged PR #10817.
Last updated: Dec 06 2025 at 07:03 UTC