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

Alternative xxhash implementation #146

Closed
nsmith- opened this issue Sep 12, 2022 · 2 comments · Fixed by #148
Closed

Alternative xxhash implementation #146

nsmith- opened this issue Sep 12, 2022 · 2 comments · Fixed by #148
Labels
bug Something isn't working evaluator Issues related to the evaluator

Comments

@nsmith-
Copy link
Collaborator

nsmith- commented Sep 12, 2022

The conda-forge feedstock cannot build v2.2.1 on non-x86 platforms conda-forge/correctionlib-feedstock#8
due to errors like

  In file included from /home/conda/feedstock_root/build_artifacts/correctionlib_1663008272199/work/src/correction.cc:10:
  /home/conda/feedstock_root/build_artifacts/correctionlib_1663008272199/work/xxhash/include/xxhash.hpp:45:10: fatal error: immintrin.h: No such file or directory
     45 | #include <immintrin.h>
        |          ^~~~~~~~~~~~~
  compilation terminated.

Perhaps https://github.com/Cyan4973/xxHash can be built for all platforms?

@nsmith- nsmith- added bug Something isn't working evaluator Issues related to the evaluator labels Sep 12, 2022
@lgray
Copy link
Contributor

lgray commented Sep 13, 2022

So - looking at it a bit. A PR to https://github.com/RedSpah/xxhash_cpp could be made to add in the proper compiler guards but it's a bit of a PITB to make sure everything is annotated correctly (and to make sure algorithms are properly done). But you can crib from Cyan4973. Judging from conda https://github.com/Cyan4973/xxHash can be built reasonably easily for all relevant platforms, but then library hell.

@lgray
Copy link
Contributor

lgray commented Sep 13, 2022

Looking even more. It seems the compiled parts of the other xxhash are fairly trivial, the .c just includes the header and there's no further meat. Shouldn't be that bad!

@nsmith- nsmith- linked a pull request Sep 13, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working evaluator Issues related to the evaluator
Development

Successfully merging a pull request may close this issue.

2 participants