Stream: git-wasmtime

Topic: wasmtime / PR #13350 riscv64: Omit sign-extension of some...


view this post on Zulip Wasmtime GitHub notifications bot (May 13 2026 at 17:30):

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 band instruction with an immediate that fits in an Imm12. 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:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2026 at 17:30):

alexcrichton requested cfallin for a review on PR #13350.

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2026 at 17:30):

alexcrichton requested wasmtime-compiler-reviewers for a review on PR #13350.

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2026 at 17:30):

alexcrichton requested wasmtime-core-reviewers for a review on PR #13350.

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2026 at 17:30):

alexcrichton updated PR #13350.

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2026 at 19:17):

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

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2026 at 22:11):

:thumbs_up: cfallin submitted PR review:

Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2026 at 22:11):

: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?

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2026 at 22:11):

: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?

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2026 at 22:12):

:memo: cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2026 at 22:12):

:speech_balloon: cfallin created PR review comment:

(The test cases would of course catch this, but I'd rather have it explicitly documented too.)

view this post on Zulip Wasmtime GitHub notifications bot (May 14 2026 at 14:59):

:memo: alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 14 2026 at 14:59):

: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