crc32: add riscv64 implementation#515
Conversation
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
folkertdev
left a comment
There was a problem hiding this comment.
Totally, riscv support just hasn't come up yet.
Thanks for adding the testing, that is kind of the main requirement I have.
Did you check that the bounds checks actually get eliminated? it looks like they should but that might need some slight tweaks.
Finally, perhaps you can add the clmul instructions to miri? You can combine ideas from
For the code changes. We usually like to validate the added test cases on real hardware, which you can actually do.
There was a problem hiding this comment.
Not needed for this PR, but I suspect an approach like in #514 could work here too? Though aarch64 has 64 x 64 -> 128 bits in one instruction, while here it takes 2 so perhaps it's not actually advantageous.
64eb473 to
7672e5e
Compare
|
Bad news, the two systems I thought had zbkc, actually don't! And worse yet, they also don't do zvv for compare256/slidehash/adler32. Not insurmountable, more modern dev boards here seem to be at reasonable cost. I'll come back to this after I source one and after my vacation. |
@folkertdev is there any interest in porting zlib-ng's riscv64 implementations into zlib-rs? I recently found myself with some riscv64 hardware, so happy to do so.
Checking if this is something you're willing to accept/maintain before I go further.