-
-
Notifications
You must be signed in to change notification settings - Fork 46
crc32: Add implementation for loongarch64 #511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| pub fn crc32_loongarch64(crc: u32, buf: &[u8]) -> u32 { | ||
| let mut c = !crc as i32; | ||
|
|
||
| // SAFETY: [u8; 8] safely transmutes into i64. | ||
| let (before, middle, after) = unsafe { buf.align_to::<i64>() }; | ||
|
|
||
| c = remainder(c, before); | ||
|
|
||
| if middle.is_empty() && after.is_empty() { | ||
| return !c as u32; | ||
| } | ||
|
|
||
| for d in middle { | ||
| c = crc_w_d_w(*d, c); | ||
| } | ||
|
|
||
| c = remainder(c, after); | ||
|
|
||
| !c as u32 | ||
| } | ||
|
|
||
| #[inline] | ||
| fn remainder(mut c: i32, mut buf: &[u8]) -> i32 { | ||
| if let [b0, b1, b2, b3, rest @ ..] = buf { | ||
| c = crc_w_w_w(i32::from_le_bytes([*b0, *b1, *b2, *b3]), c); | ||
| buf = rest; | ||
| } | ||
|
|
||
| if let [b0, b1, rest @ ..] = buf { | ||
| c = crc_w_h_w(i16::from_le_bytes([*b0, *b1]), c); | ||
| buf = rest; | ||
| } | ||
|
|
||
| if let [b0, rest @ ..] = buf { | ||
| c = crc_w_b_w(*b0 as i8, c); | ||
| buf = rest; | ||
| } | ||
|
|
||
| debug_assert!(buf.is_empty()); | ||
|
|
||
| c | ||
| } | ||
|
|
||
| crate::cfg_select! { | ||
| miri => { | ||
| use core::arch::loongarch64::{crc_w_b_w, crc_w_h_w, crc_w_w_w, crc_w_d_w}; | ||
| } | ||
| _ => { | ||
| use asm::{crc_w_b_w, crc_w_h_w, crc_w_w_w, crc_w_d_w}; | ||
| } | ||
| } | ||
|
|
||
| // FIXME: there are intrinsics for these in the standard library, but currently | ||
| // unstable behind the stdarch_loongarch feature | ||
| // | ||
| // CRC32 instructions are part of the basic integer operations and therefore | ||
| // always available. | ||
| mod asm { | ||
| /// CRC32 single round checksum for bytes (8 bits). | ||
| /// | ||
| /// [Loongson's documentation](https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#crc-check-instructions) | ||
| pub fn crc_w_b_w(data: i8, mut crc: i32) -> i32 { | ||
| unsafe { | ||
| core::arch::asm!( | ||
| "crc.w.b.w {crc}, {data}, {crc}", | ||
| crc = inout(reg) crc, | ||
| data = in(reg) data, | ||
| options(pure, nomem, nostack, preserves_flags) | ||
| ); | ||
| } | ||
| crc | ||
| } | ||
|
|
||
| /// CRC32 single round checksum for half words (16 bits). | ||
| /// | ||
| /// [Loongson's documentation](https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#crc-check-instructions) | ||
| pub fn crc_w_h_w(data: i16, mut crc: i32) -> i32 { | ||
| unsafe { | ||
| core::arch::asm!( | ||
| "crc.w.h.w {crc}, {data}, {crc}", | ||
| crc = inout(reg) crc, | ||
| data = in(reg) data, | ||
| options(pure, nomem, nostack, preserves_flags) | ||
| ); | ||
| } | ||
| crc | ||
| } | ||
|
|
||
| /// CRC32 single round checksum for words (32 bits). | ||
| /// | ||
| /// [Loongson's documentation](https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#crc-check-instructions) | ||
| pub fn crc_w_w_w(data: i32, mut crc: i32) -> i32 { | ||
| unsafe { | ||
| core::arch::asm!( | ||
| "crc.w.w.w {crc}, {data}, {crc}", | ||
| crc = inout(reg) crc, | ||
| data = in(reg) data, | ||
| options(pure, nomem, nostack, preserves_flags) | ||
| ); | ||
| } | ||
| crc | ||
| } | ||
|
|
||
| /// CRC32 single round checksum for double words (64 bits). | ||
| /// | ||
| /// [Loongson's documentation](https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#crc-check-instructions) | ||
| pub fn crc_w_d_w(data: i64, mut crc: i32) -> i32 { | ||
| unsafe { | ||
| core::arch::asm!( | ||
| "crc.w.d.w {crc}, {data}, {crc}", | ||
| crc = inout(reg) crc, | ||
| data = in(reg) data, | ||
| options(pure, nomem, nostack, preserves_flags) | ||
| ); | ||
| } | ||
| crc | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So here is an idea: rust-lang/miri#4899 added crc32 support for aarch, you could add similar support for loongarch? A requirement for new shims is often that you actually check the results on real hardware, but that is something you can actually do.
With miri support we can actually test this in CI today if instead of asm you use the intrinsics here. We can add some
__internalfeature for it if needed.Separately we could actually stabilize the crc intrinsics on loongarch if the target maintainer is OK with that. For signatures we follow the clang headers by default (but it's ultimately up to the target maintainer), hence the i32 instead of more accurate widths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, adding support for these in Miri sounds good. I'll try to put together a PR this evening.
So you would be okay with requiring nightly behind a feature flag?
Funny enough, the clang headers do have different (in my view, better) signatures (see https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/larchintrin.h#L60) and use i8 and i16 where appropriate. GCC also seems to do this better (https://github.com/gcc-mirror/gcc/blob/master/gcc/config/loongarch/larchintrin.h#L152). I'll submit a patch to align the LLVM intrinsics with clang and GCC. Afterwards I think stabilization would be reasonable, though I've also noticed that LLVM completely prohibits using the CRC intrinsics on 32-bit, while Rust currently exposes them on both 32-bit and 64-bit, which should cause compile errors. I'm not sure whether that's a hardware limitation or not and what is actually correct in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already do for
gzprintf, which will just become a default-enabled feature when our msrv reaches whatever version c-variadic will be stable in. We're conservative with bumping the msrv but c-variadic and custom allocators are enticing reasons to bump.For now, given that I don't think there is a really urgent use case I'd say add it as an
__internal_loongarch_crc32feature, we can give it a proper name once we have a bit more clarity on the testing and stabilization situation.cc @heiher on the crc questions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm actually, now that I'm looking at it (after I'm hitting operand promotion LLVM crashes when changing the LLVM intrinsics), the aarch64 intrinsics are also defined with
i32in LLVM. The stdarch wrappers around them simply perform the cast, similar to how clang does it. I guess this doesn't need changes in LLVM, only in stdarch.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rust-lang/stdarch#2136
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just hope you don't blindly trust Miri for correctness of these operations -- I don't know enough about their intended behavior to make strong promises.^^
OTOH if you are testing both on real HW and Miri and they give the same result, that helps us ensure Miri is correct. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it's the same idea as avx512, which we run/fuzz/ occasionally on the actual hardware. The CI job also provides some protection against rust/stdarch/llvm doing something weird.