Shim the LoongArch CRC intrinsics#5062
Conversation
|
Thank you for contributing to Miri! A reviewer will take a look at your PR, typically within a week or two. |
There was a problem hiding this comment.
Thanks a lot, just a few comments. :)
The code (and the test) are slightly ugly because the intrinsics take signed integers. I'd be very happy about suggestions to make it prettier.
What do you mean, you don't seem to be doing anything about the sign and just interpret the inputs as unsigned? (In the code, I mean. For the tests I guess it'd be up to stdarch to make the API less annoying...)
LLVM's intrinsic definitions: https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/IR/IntrinsicsLoongArch.td#L78-L94 (note they all take i32 or i64 parameters)
LLVM doesn't distinguish signed and unsigned integers so that does not mean much
@rustbot author
|
Reminder, once the PR becomes ready for a review, use |
Yeah, I did change that in a follow-up commit since because CI was complaining about
TIL! @rustbot ready |
Okay, please update the PR description. :) |
Done! |
|
Wait, we aren't even running these tests, are we? We'll have to add a loongarch target to CI. I'll take care of that. |
I'm adding support for using these in zlib-rs, and @folkertdev suggested implementing them in Miri (trifectatechfoundation/zlib-rs#511 (comment)). The LoongArch CRC intrinsics are extremely similar to those found on AArch64, with the main difference that the argument order is swapped.
Here's the test in Miri and on actual hardware:
i.e. it behaves the same on real hardware and in Miri.
The implementation is based on the ACLE code with only very slight adjustments; the test is also basically the same one.
Helpful links for reviewing:
i32ori64parameters)