UoB-HPC / SimEng

The University of Bristol HPC Simulation Engine
https://uob-hpc.github.io/SimEng
Apache License 2.0
93 stars 20 forks source link

Corrected RISC-V shift word instructions #412

Closed JosephMoore25 closed 6 months ago

JosephMoore25 commented 6 months ago

I spotted that the RISC-V instructions for shifting words have a minor inaccuracy. In the 64-bit variants, we take the lowest 6 bits (up to 63) for the shamt (shift-amount). In the word (32 bit) variants, we should only be taking the lowest 5 bits (up to 31) for shamt, as there are only 31 bit positions to shift before you've looped back.

Functionally, this inaccuracy caused no issues due to the cyclic nature of shifts. If you are to shift a 32 bit number by shamt=34, this is equivalent to shamt=2. Due to this, all tests previously passed and still do pass. For the sake of being spec accurate, this change should be made to limit the shifts of words to 31. No test can be written to cover this due to them being functionally identical.

I've verified for each instruction that this is correct compared to the RISC-V unprivileged spec, SAIL, and the spike implementation of these instructions.

From the spec:

In RV64I, only the low 6 bits of rs2 are considered for the shift amount. SLLW, SRLW, and SRAW are RV64I-only instructions that are analogously defined but operate on 32-bit values and produce signed 32-bit results. The shift amount is given by rs2[4:0].