-
-
Notifications
You must be signed in to change notification settings - Fork 158
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
Enable macos universal2
build with AVX2/SSE2 optimisation for x86_64 target
#347
Conversation
… AVX2 or SSE2 is not available
CI's broken for |
So if I understand (If that is the case, the changes to shuffle.c are unnecessary) |
In addition to @kalvdans comment, I'd suggest to use (or plan to adopt) C-Blosc2 for creating universal binaries, as it has native support for NEON in ARM machines. C-Blosc2 brings other interesting features, and is backward compatible with C-Blosc1. |
Yeah, that's unfortunate and should be fixed somehow. |
Yes a single call to
Something needs to be done in |
+1! |
I would argue that those structures should not exist on ARM. |
Thus the use of |
Got you! I now understand your PR and I'm in favour of the changes. |
blosc/bitshuffle-avx2.c
Outdated
@@ -19,8 +19,8 @@ | |||
|
|||
/* Make sure AVX2 is available for the compilation target and compiler. */ | |||
#if !defined(__AVX2__) | |||
#error AVX2 is not supported by the target architecture/platform and/or this compiler. | |||
#endif | |||
#warning AVX2 is not supported by the target architecture/platform and/or this compiler. |
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.
Don't print a warning, it is annoying to have to ignore it and it fails to compile with -Werror
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 removed them.
It occurs to me that it should be relatively easy for C-Blosc to understand chunks from C-Blosc2 (besides the additional 16 bytes in the Blosc2 header, they are largely compatible). Would that help your migration plans? |
Not sure, or as a transition (but does it worth it? and is it the way to go with HDF5 filters?). |
Hi, Do you plan on tagging a version of |
Yes, I'd like to cut a release of C-Blosc soon. But no need to wait for me; go ahead and just reference the merge commit. |
This PR proposes a fix to allow building
c-blosc
as part of a Python package that is built as auniversal2
binary package on macos with enabled AVX2/SSE2 optimisation for the x86_64 target only.To achieve this,
c-blosc
should support definingSHUFFLE_AVX2_ENABLED
andSHUFFLE_SSE2_ENABLED
and be able to build all files (including*-avx2.c
and*-sse2.c
) even when AVX2/SSE2 are not supported.This is done by toggling AVX2/SSE2 with both
SHUFFLE_AVX2_ENABLED
,SHUFFLE_SSE2_ENABLED
and__AVX2__
,__SSE2__
.Related to #334
Related to silx-kit/hdf5plugin#186