alexcrichton requested elliottt for a review on PR #7045.
alexcrichton requested wasmtime-compiler-reviewers for a review on PR #7045.
alexcrichton opened PR #7045 from alexcrichton:refactor-riscv64-immediates
to bytecodealliance:main
:
I was curious to poke around the riscv64 backend and I wanted to touch up the handling of
Imm{12,20}
a bit after reading it. This commit is a refactoring of these two types with the following changes:
The payload of these types is now unsigned and guarantees that irrelevant bits are set to zero. For example
Imm12
is stored asu16
where the upper four bits are guaranteed to be zero. This fixes a discrepancy whereImm12::maybe_from_i64
was masked for example butImm12::from_bits
wasn't.The
Neg for Imm12
impl was removed because -2048 is a validImm12
but 2048 is not in-range forImm12
meaning that it is not an infallible operation.Accessors are now named
bits
to get theu32
representation suitable to be encoded into an instruction. Acquiring the underlying value is now done withas_i{16,32}
depending on the type. The signed accessor does sign-extension as required to produce the semantically equivalent value.Manual constructors were renamed to
from_{i16,i32}
instead offrom_bits
. This in theory helps convey that they're constructors for logical values rather than literal bit-wise values. Additionally asserts are now placed in these constructors asserting that the provided value is in-range.The
FALSE
andTRUE
constants were renamedZERO
andONE
andImm20::ZERO
was added.This commit ended up changing many runtests, but only their CLIF printing rather than their encoding. This change is due to the fact that
Display
now prints the logical value of the immediate rather than the raw bit representation as a base 10 integer. It's not intended that this commit actually changes any behavior, instead it should purely be internal refactorings.<!--
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 updated PR #7045.
afonso360 submitted PR review:
LGTM! :+1: Thank you for looking into this. The Imm12 printing as unsigned has been slightly bothering me for a while now!
alexcrichton has enabled auto merge for PR #7045.
alexcrichton updated PR #7045.
alexcrichton merged PR #7045.
Last updated: Dec 23 2024 at 12:05 UTC