openhwgroup / corev-binutils-gdb

GNU General Public License v2.0
9 stars 26 forks source link

cv.avgu and cv.srl/sra/sll instructions out of sync #96

Closed melonedo closed 5 months ago

melonedo commented 11 months ago

In https://github.com/openhwgroup/cv32e40p/commit/f2a1997cfd2379ca09ed89795ec4eb25cec9c1e7, a change of spec demands that the immediate argument of shift instructions no longer allow values greater than the number of bits in a register, while the testsuite still tests for values out of this range:

https://github.com/openhwgroup/corev-binutils-gdb/blob/8c3d27d122bdf3aab4205c0be7705ebe8db28bbf/gas/testsuite/gas/riscv/cv-simd-srl-sci-b-pass.d#L18

Similarly, https://github.com/openhwgroup/cv32e40p/commit/cdee092d6413ef0af53c63ac7eaf1d075be864ea changes cv.avgu to zero-extension(unsigned), while the testsuite still tests for negative values:

https://github.com/openhwgroup/corev-binutils-gdb/blob/8c3d27d122bdf3aab4205c0be7705ebe8db28bbf/gas/testsuite/gas/riscv/cv-simd-avgu-sci-b-pass.d#L17

melonedo commented 11 months ago

The second part was fixed in #97. To clarify the first part, the spec for cv.srl states in https://github.com/openhwgroup/cv32e40p/commit/f2a1997cfd2379ca09ed89795ec4eb25cec9c1e7:

Only Imm6[3:0] and rs2[3:0] are used for .h instruction and Imm6[2:0] and rs2[2:0] for .b instruction. Other bits are not used and must be set to 0.

Thus setting Imm6 to 63 should not be allowed for cv.srl.

MaryBennett commented 5 months ago

AP: check the manual. Should now be expecting an error.

MaryBennett commented 5 months ago

Merged fix. Closing issue.