-
Notifications
You must be signed in to change notification settings - Fork 330
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
AddLower, PairwiseAdd/Sub and MaskedAbsOr operations #2405
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Thanks for adding! Are you able to sign the CLA? On the naming: Lower typically references the lower half. How about we rename AddLower to AddLane, similar to GetLane? One concern about constexpr for Pairwise128Indices: this seems to require C++17, right? |
Thanks @jan-wassenberg for a quick feedback. We will address your comments. Regarding the CLA, someone from our org signed it and should have included my teams (including mine) email Ids in the CLA. For some reason it has not worked. Let me check if we need to do more from our end. |
FYI the CLA check does mention the email "Author: @mazimkhan <az*****an@cambridgeconsultants.com>" which looks correct. |
For 16-byte I16/U16/I32/U32/F32 vectors on SSSE3/SSE4/AVX2/AVX3/AVX10, For 32-byte I16/U16/I32/U32/F32 vectors on AVX2/AVX3/AVX10, |
Here is an improvement to the implementation of PairwiseAdd128/PairwiseSub128:
|
Remove AddLower, MaskedAddOr can be used instead Rename MaskedAbsOrZero and reorder MaskedAbsOr args
Co-authored-by: John Platts <[email protected]>
cc5c4e8
to
f36d5f5
Compare
@mazimkhan Could you please go ahead in the changes that I made in the hwy_pairwise_add_enh_013025 branch of the https://github.com/johnplatts/jep_google_highway repository (which are contained in commit johnplatts@ed95fe6)? Here are the git commands for merging the changes in commit johnplatts@914cb69:
|
@johnplatts Your update seems to have an issue with compiling for SSE2. |
Yes, the HADD is not available on SSE2, we can make it conditional on HWY_TARGET < HWY_SSE2. |
@wbb-ccl I have made a change to hwy/ops/x86_128-inl.h the hwy_pairwise_add_enh_013025 branch, and those changes can be found in commit johnplatts@61804b1. |
@wbb-ccl I made some additional changes to arithmetic_test.cc in commit johnplatts@c446e6f of the hwy_pairwise_add_enh_013025 branch to fix compiler warnings with earlier versions of GCC. |
Adding special arithmetic operations for
arm_sve-inl.h
andgeneric_ops-inl.h
:Tests have been added for the operations.
The instruction matrix in g3doc/instruction_matrix.pdf may need to be updated, but it appears to have been generated manually.