Skip to content

crc32: add riscv64 implementation#515

Closed
inahga wants to merge 2 commits into
trifectatechfoundation:mainfrom
inahga:inahga/crc32-riscv64
Closed

crc32: add riscv64 implementation#515
inahga wants to merge 2 commits into
trifectatechfoundation:mainfrom
inahga:inahga/crc32-riscv64

Conversation

@inahga
Copy link
Copy Markdown
Contributor

@inahga inahga commented May 29, 2026

@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.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2026

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
zlib-rs/src/cpu_features.rs 0.00% 3 Missing ⚠️
Flag Coverage Δ
fuzz-compress ?
fuzz-decompress ?
test-aarch64-apple-darwin 87.89% <0.00%> (-0.03%) ⬇️
test-aarch64-unknown-linux-gnu 85.27% <0.00%> (-0.03%) ⬇️
test-i686-unknown-linux-gnu 84.98% <0.00%> (-0.01%) ⬇️
test-x86_64-apple-darwin 88.21% <0.00%> (-0.02%) ⬇️
test-x86_64-unknown-linux-gnu 89.88% <0.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
zlib-rs/src/crc32.rs 94.94% <ø> (ø)
zlib-rs/src/cpu_features.rs 60.97% <0.00%> (-4.82%) ⬇️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@folkertdev folkertdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread zlib-rs/src/crc32/zbc.rs Outdated
Comment thread zlib-rs/src/cpu_features.rs Outdated
Comment thread zlib-rs/src/crc32/zbkc.rs
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread zlib-rs/src/crc32/zbc.rs Outdated
@inahga inahga force-pushed the inahga/crc32-riscv64 branch from 64eb473 to 7672e5e Compare June 2, 2026 23:54
@inahga
Copy link
Copy Markdown
Contributor Author

inahga commented Jun 3, 2026

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.

@inahga inahga closed this Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants