Add support for AArch64 SHA extensions. #398
Conversation
|
OK, now a new error: This is when running |
|
Oh wait, I probably need to set target_feature |
|
Nope, that wasn't it. |
|
Try with |
coresimd/aarch64/neon.rs
Outdated
| #[link_name = "llvm.aarch64.neon.fminv.f64.v2f64"] | ||
| fn vminvq_f64_(a: float64x2_t) -> f64; | ||
|
|
||
| #[link_name = "llvm.arm.neon.sha1h"] |
There was a problem hiding this comment.
llvm.arm.neon hints that they do belong in the coresimd/arm/neon.rs module
There was a problem hiding this comment.
OK, good! I wasn't sure if crypto.rs would make more sense.
coresimd/aarch64/neon.rs
Outdated
|
|
||
| /// SHA1 hash update accelerator, choose. | ||
| #[inline] | ||
| #[target_feature(enable = "neon")] |
There was a problem hiding this comment.
have you tried enabling "neon,crypto" here ?
There was a problem hiding this comment.
they probably just require the crypto feature
There was a problem hiding this comment.
Yeah, (I did #[target_feature(enable = "neon", enable = "crypto")]). No luck.
Didn't help, unfortunately. |
|
So I'll give these a try tomorrow (gotta sleep now), but what you can do in the meantime is check what clang does for these, in particular, clang / llvm tests, what features, or -mattr do they enable to compile the tests for these intrinsics, and try those. |
|
Turns out I should have been using |
coresimd/aarch64/crypto.rs
Outdated
| #[inline] | ||
| #[target_feature(enable = "crypto")] | ||
| #[cfg_attr(test, assert_instr(sha1su0))] | ||
| pub unsafe fn vsha1su0q_u32(w0_3: u32x4, w4_7: u32x4, w8_11: u32x4) -> u32x4 { |
There was a problem hiding this comment.
I think these should use the uint32x4_t types nowadays, right @gnzlbg?
There was a problem hiding this comment.
Makes sense. Fixed.
|
Thanks! Mind running rustfmt? After that I think this is good to go |
|
I also added AES for completeness. Note that there are currently no tests (other than assert_instr). |
jasondavies
changed the title
|
@alexcrichton let's wait for the tests here :D But @jasondavies this looks really good, keep it up! |
|
Oh oops sorry, I misread! |
|
Tests added. |
| let r: u8x16 = vaeseq_u8(data, key).into_bits(); | ||
| assert_eq!( | ||
| r, | ||
| u8x16::new( |
There was a problem hiding this comment.
nitpick: no need to do this, but if you want to manually format these next time just put them in a variable and tell rustfmt to skip these:
#[cfg_attr(rustfmt, skip)]
let r = u8x16::new(
1, 2, 3, 4, 5, 6, 7, 8
9, 10, 11, 12, 13, 14, 15
);
|
LGTM. I've restarted the SDE build bot in case the failure was spurious, and the rustfmt build bot is failing (maybe you need to |
|
Thanks, I did notice that but wasn't sure if I should be changing so many files just to satisfy rustfmt for this PR. Just pushed a commit that should fix it, after running |
|
Thanks! I think that recent nightlies have broken CI here for i128, mind removing that feature listed while you're at it? |
|
Hm a new test is failing, I wonder if that's related to the formatting changes by accident? |
|
Maybe I should revert the rustfmt commit and see if it passes? |
|
Sure! |
|
Which test is failing? The Intel SDE build bot was already failing before the formatting changes, and AFAICT this PR only touches AArch64 anyways :/ |
|
Yeah it was just the SDE one. I guess I should just kill the last revert commit in that case and this should be ready to merge. |
|
The formatting changes touch a whole bunch of files but I agree it would be a bit worrying if formatting changes caused any test failures. |
|
OK, done. |
|
This is the build bot where SDE is running without the formatting changes: https://travis-ci.org/rust-lang-nursery/stdsimd/jobs/358879191 If that still fails, let's just merge this and fix SDE afterwards. Maybe something on nightly has changed that breaks SDE, or maybe we just need to update SDE to a newer version, but none of these are this PRs fault. |
|
Looks like the SDE build with no formatting changes failed. |
|
The last PR was 4 days ago and back then everything was green so something in the last couple of nightlies might have broken it :/ |
3 participants
No description provided.