Skip to content

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