Skip to content
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

Add VPCLMULQDQ Intrinsics #943

Merged
merged 2 commits into from
Dec 1, 2020
Merged

Add VPCLMULQDQ Intrinsics #943

merged 2 commits into from
Dec 1, 2020

Conversation

DevJPM
Copy link
Contributor

@DevJPM DevJPM commented Nov 1, 2020

This adds support for the two intrinsics covered by VPCLMULQDQ
and extends the tests from PCLMULQDQ as well as compares the results of these two
when the inputs on the lanes are different

Like #942 this depends on rust-lang/rust#78361 to work (though depending on avx512pclmulqdq) and was tested with that patch applied.
The test vectors are the same as pclmulqdq.rs.
The folder is chosen to match AVX512F / -IFMA.
The name is chosen as detected in Rust.
I have marked this PR as a draft until the required compiler change landed in nightly.

@rust-highfive
Copy link

r? @Amanieu

(rust_highfive has picked a reviewer for you, use r? to override)

@DevJPM DevJPM changed the title Added support for VPCLMULQDQ Extension Add VPCLMULQDQ Intrinsics Nov 1, 2020
@DevJPM DevJPM marked this pull request as ready for review November 20, 2020 09:14
@DevJPM
Copy link
Contributor Author

DevJPM commented Nov 20, 2020

OK, I'm confused now. When I ask only for the short names of the intrinsics (i.e. vpclmulqdq) it fails not finding the long ones (e.g. vpclmullqlqdq). But when I ask for the long ones as well, as the pclmulqdq.rs file does, it fails sometimes because the disassembly only contains the long name.

@bors
Copy link
Contributor

bors commented Nov 22, 2020

☔ The latest upstream changes (presumably 7a533ec) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@Amanieu
Copy link
Member

Amanieu commented Nov 22, 2020

I think both disassemblies are valid, but you can put a common prefix in assert_instr so that it will match both.

@DevJPM DevJPM force-pushed the vpclmul branch 2 times, most recently from de491ee to d586a3e Compare November 22, 2020 15:10
This adds the vectorized PCLMULQDQ extensions.
Test vectors are the same as for the old version or compare outputs.
@DevJPM
Copy link
Contributor Author

DevJPM commented Nov 22, 2020

Sorry for all the force-pushed, I majorly screwed up the rebase from master (which was forced by me using the same location in mod.rs for VAES and VPCLMUL).

Also thank you very much for the prefix-advice. I hadn't realized that it only checks prefixes / substrings.

@Amanieu Amanieu merged commit b3b03be into rust-lang:master Dec 1, 2020
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.

4 participants