guzba / zippy

Pure Nim implementation of deflate, zlib, gzip and zip.
MIT License
246 stars 29 forks source link

Signed shifts caused problems on 64-bit RISC-V #4

Closed fwsGonzo closed 3 years ago

fwsGonzo commented 3 years ago

This is not a call to arms, just something for me to remember that I need to look more into this. I am able to use 32-bit RISC-V just fine with zippy, but the signed shifts cause problems on 64-bit.

So what gives? Well, on RISC-V the arithmetic shifts (SRAI instructions primarily) will drag the sign bit with them down. So any time you have a signed value it will still be signed afterwards. Example (just imagine that these are 64-bit signed integers instead of 16-bit): 0x8000 SRAI 8 == 0xFF80 0x8000 SRAI 12 == 0xFFF8 0x8000 SRAI 15 == 0xFFFF

Exactly which lines of code cause SRAI to be emitted is not known to me, but I believe it wouldn't be emitted for unsigned shifts.

guzba commented 3 years ago

I'm thinking this may be related to this:

https://github.com/guzba/zippy/blob/master/src/zippy/lz77.nim#L62-L63

Which was previously a signed shift but should be unsigned now.

There are other signed shifts I am sure. As someone who's done a bit with RISC-V, how would you suggest I do some testing myself? I know I can dig into it but maybe you can get me on the right track faster :)

RSDuck commented 3 years ago

this doesn't seem to be RISC-V specific in any way. Every processor which uses twos complement to represent signed integers (so pretty much every processor) I know has a signed and an unsigned right shift instruction.

C compilers usually emit arithmetic (signed) right shifts for right shifting a signed type and unsigned right shift instructions for shifting right unsigned numbers.

guzba commented 3 years ago

Closing this as I believe it is resolved. Please re-open there is a reproducible issue I can investigate.