alexcrichton opened PR #13350 from alexcrichton:riscv64-opts to bytecodealliance:main:
This is a minor optimization for the riscv64 backend where a sign/zero extension operation can be skipped for a
bandinstruction with an immediate that fits in anImm12. In this situation, with a positive immediate, the instruction generated will already zero extend the immediate which guarantees the sign bit and all upper bits are 0. In this situation no further extension is needed.<!--
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
-->
alexcrichton requested cfallin for a review on PR #13350.
alexcrichton requested wasmtime-compiler-reviewers for a review on PR #13350.
alexcrichton requested wasmtime-core-reviewers for a review on PR #13350.
alexcrichton updated PR #13350.
github-actions[bot] added the label cranelift on PR #13350.
:thumbs_up: cfallin submitted PR review:
Thanks!
:speech_balloon: cfallin created PR review comment:
This seems fine as an optimization but is definitely a coupling-at-a-distance with the other lowering rule -- e.g. if we ever had another rule take precedence, or a change for some other reasoning to our
band-of-imm12-compatible-immediate case, this might no longer be true. Can we add a corresponding comment to that lowering rule noting that it is load-bearing for correctness w.r.t.val_already_extended?
:speech_balloon: cfallin created PR review comment:
can we add a few cases to exercise the full range of the
imm12(and just beyond) to lock in our reasoning about the sign-bits?
:memo: cfallin submitted PR review.
:speech_balloon: cfallin created PR review comment:
(The test cases would of course catch this, but I'd rather have it explicitly documented too.)
:memo: alexcrichton submitted PR review.
:speech_balloon: alexcrichton created PR review comment:
We've definitely run into this in the past too with the x64 backend as well. Personally I'm a bit ambivalent whether optimizations like this are worth it. This is just instruction golfing, I don't actually have any performance data.
IIRC in the past with x64 we've decided to not implement optimizations like this in favor of some sort of future hypothetical peephole optmization pass or something like that. Personally I feel like this might fall into that category for riscv64, and even the above matchings for arithmetic instructions in theory should be removed in favor of such a peephole pass as well. I don't think it's worth implementing such a peephole pass at this time though.
Last updated: Jun 01 2026 at 09:49 UTC