Skip to content

Support SIMD Histogram Subroutines on aarch64 #2447

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Support SIMD Histogram Subroutines on aarch64 #2447

wants to merge 2 commits into from

Conversation

wx257osn2
Copy link
Contributor

Currently SIMD histogram subroutines are written with simdlib and AVX2 intrinsics .
This PR adds some functions to simdlib and removes AVX2 intrinsics from SIMD histogram subroutines, so faiss with this PR can execute histogram using ARM SIMD on aarch64.

@wx257osn2 wx257osn2 marked this pull request as ready for review September 1, 2022 21:09
@wx257osn2
Copy link
Contributor Author

wx257osn2 commented Sep 1, 2022 •
edited
Loading

Ah, ok, this PR depends on #2448. I'll re-send this PR after #2448 will be merged. Sorry for sending you an undesirable notification... 🙇

@wx257osn2 wx257osn2 closed this Sep 1, 2022
@wx257osn2 wx257osn2 reopened this Sep 8, 2022
@mdouze
Copy link
Contributor

mdouze commented Sep 13, 2022

The code looks good.
However, the histogram functionality is not used in the current version of the FastScan implementation, therefore I am not sure there is test coverage for the function. Could you check that? And otherwise add a test?

@mdouze
Copy link
Contributor

mdouze commented Sep 13, 2022

Ah right, it is tested in
https://github.com/facebookresearch/faiss/blob/151e3d7be54aec844b6328dc3e7dd0b83fcfa5bc/tests/test_partition.py
so that looks fine.

@facebook-github-bot
Copy link
Contributor

@mdouze has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@mdouze
Copy link
Contributor

mdouze commented Oct 6, 2022

I have trouble landing this, I may need to make a new PR

@wx257osn2
Copy link
Contributor Author

@mdouze Is there any action that I should do, like closing this and making a new PR with a new branch, or something like that?

@mdouze
Copy link
Contributor

mdouze commented Oct 7, 2022

If you could rebase this PR on the latest Faiss it would be helpful. Thansk.

@wx257osn2
Copy link
Contributor Author

@mdouze Sorry, I thought it was rebased by the "Update branch" button until just now (the button had been MERGING main branch into this branch, but not rebasing this branch into main) 🙇 I have rebased it, so would you import it again? thanks

@facebook-github-bot
Copy link
Contributor

@mdouze has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@mdouze
Copy link
Contributor

mdouze commented Mar 24, 2023

Thanks, second attempt...

@facebook-github-bot
Copy link
Contributor

@mdouze merged this pull request in 5dcd282.

@wx257osn2 wx257osn2 deleted the histogram-neon branch March 24, 2023 13:05
BZO95 added a commit to BZO95/faiss that referenced this pull request Apr 10, 2025
@BZO95
Support SIMD Histogram Subroutines on aarch64 (#2447)
581e2c8 
Summary:
Currently SIMD histogram subroutines are written with `simdlib` **and AVX2 intrinsics** .
This PR adds some functions to `simdlib` and removes AVX2 intrinsics from SIMD histogram subroutines, so faiss with this PR can execute histogram using ARM SIMD on aarch64.

Pull Request resolved: facebookresearch/faiss#2447

Reviewed By: alexanderguzhva

Differential Revision: D39461155

Pulled By: mdouze

fbshipit-source-id: 63a664e3e2ed462b451acc346ea58a2532f294c9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wx257osn2 @mdouze @facebook-github-bot