-
Notifications
You must be signed in to change notification settings - Fork 283
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
Swap blake3 to FetchContent + general cleanup #402
Conversation
cmmarslender
commented
Oct 13, 2023
•
edited
Loading
edited
- Gets blake3 via fetchcontent in cmake
- Builds windows wheels with cmake as well, vs the old split method used in setup.py
- re-enables the tests on windows which were disabled when bladebit compress harvesting was merged
- Fix edge case in C++ tests on mac and add mac C++ tests to CI
…vs small string optimization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only question is - now that we're not specifying the assembly versions, will Windows still get the CPU optimized code?
it appears they handle this all on their end now (they added cmake config since we initially pulled in the files). As another example, they have this output that shows up when assembly is enabled: https://github.com/BLAKE3-team/BLAKE3/blob/master/c/CMakeLists.txt#L180 Which we can see in our windows build here: https://github.com/Chia-Network/chiapos/actions/runs/6594714279/job/17918903037?pr=402#step:3:40 Can also see the NEON stuff enabled automatically in the ARM builds: https://github.com/Chia-Network/chiapos/actions/runs/6594714283/job/17918903167?pr=402#step:8:285 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I now remember Amine making those changes.