Error in implementation of SRAW and SRAIW instructions

[Nanotrust]

Hi, I found inconsistent results when using the shift instructions sraw and sraiw. These 64-bit-specific instructions perform the shift on the right-hand 32-bit part of the word and propagate the sign of this 32-bit word to all bits, those added on the left-hand side of the 32-bit word as well as on the left-hand 32-bit part of the 64-bit word. The current implementation doesn't seem to manage the propagation of this sign, which corresponds to the 32nd bit of the 64-bit word. An example of the result for a 3-bit arithmetic right shift on 32-bit word in RV64I (the 32th bit is 0 and noted between parenthesis in unsigned binary representation) :

input :  -11110001111000111101001001011001101010001100001111011011101
input length:  60
input unsigned binary :  11111000011100001110000101101101(0)0110010101110011110000100100011
input binary length :  64
output :  1110000000000000000000000000000000000110010101110011110000100100
output length :  64

As you could see "111" at the beginning should be "000" in the output. A formal description of those instruction can be found here https://github.com/SymbioticEDA/riscv-formal/blob/master/insns/insn_sraw.v

In the ShiftPlugin, the problem seems to be at line : https://github.com/SpinalHDL/NaxRiscv/blob/8816542dc59579e82e419ecf93586215dcb3b902/src/main/scala/naxriscv/execute/ShiftPlugin.scala#L69 I think that a specific line when IS_W_RIGHT is true should be added :

shifted := (S((SIGNED & ss.SRC1(31)) ## reversed) >> amplitude).resize(width bits)

Vexiiriscv has the same issue.

[Dolu1990]

Hi,

I just ran that code in a VexiiRiscv simulation :

    li a0, 0xF870E16D32B9E123
    sraiw a1, a0, 3
    li a2, 0x6573C24
    bne a1, a2, fail

And it seems it is behaving as expected. I can see the CPU writing the 64 bits 0x6573C24 as a result

Did you observed the behaviour in a Nax / Vexii simulation ?

Did you noticed https://github.com/SpinalHDL/VexiiRiscv/blob/dev/src/main/scala/vexiiriscv/execute/BarrelShifterPlugin.scala#L53 ? The idea is that the sign extends is handled futher down in a centralized way, not directly in the BarrelShifterPlugin

[Nanotrust]

ok, I get the idea. Thanks

[Dolu1990]

doesn't seem that you've implemented the sign extension in a centralized way on the nax

It should be in, see https://github.com/SpinalHDL/NaxRiscv/blob/main/src/main/scala/naxriscv/execute/ShiftPlugin.scala#L57

To be clear, this kind of "stuff" isn't SpinalHDL, but some "regular" scala code, which is used for hardware elaboration time :)