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

Add runtime checks for use of SIMD implementations and stubs functions if not available. #1

Merged
merged 6 commits into from
Jul 3, 2024

Conversation

t20100
Copy link

@t20100 t20100 commented Jul 3, 2024

This PR builds on Blosc#622 and adds stubs for shuffle and bitshuffle functions when a given SIMD is not available at runtime.

This aims at not relying only on SHUFFLE_*_ENABLED macros to ensure a SIMD implementation is available, but also to rely on the effective SIMD macros (e.g., __AVX2__) while not compile shuffle.c with SIMD enabled.
To do so, this PR adds a is_shuffle_* (or is_bshuf_*) const bool to each implementation which is set at build time according to the SIMD macros.
This const bool is checked at runtime to check that a given SIMD implementation is available.

is_shuffle_* and is_bshuf_* are currently doing the same checks, so only one should be needed. I preferred to use both to have each source file declaring its availability.

I've checked that it works with macos universal2 builds.

@@ -592,4 +592,23 @@ int64_t bshuf_untrans_bit_elem_altivec(const void* in, void* out, const size_t s
return count;
}

#endif /* defined(__ALTIVEC__) */

const bool is_bshuf_altivec = true;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used naming consistent with the functions.
The naming (like the SIMD suffix) could be made consistent between bitshuffle and shuffle functions

Comment on lines 23 to 26
#include "bitshuffle-avx512.h"
#include "bitshuffle-avx2.h"
#include "bitshuffle-sse2.h"
#include "bitshuffle-generic.h"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved those includes outside of #if defined... to be consistent with other SIMD implementations

@@ -22,13 +22,13 @@

#include "bitshuffle-neon.h"
#include "bitshuffle-generic.h"
#include <stdlib.h>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move stdlib.h include outside of #if defined, it is also needed for abort

@@ -91,7 +91,8 @@ typedef enum {

/* Detect hardware and set function pointers to the best shuffle/unshuffle
implementations supported by the host processor. */
#if defined(SHUFFLE_AVX2_ENABLED) || defined(SHUFFLE_SSE2_ENABLED) /* Intel/i686 */
#if (defined(SHUFFLE_AVX2_ENABLED) || defined(SHUFFLE_SSE2_ENABLED)) && \
(defined(__i386__) || defined(__x86_64__) || defined(_M_IX86) || defined(_M_X64)) /* Intel/i686 */
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the check of architecture since for universal2 build this part which looks to be x86 only was compiled for arm as well (since SHUFFLE_AVX2_ENABLED and SHUFFLE_SSE2_ENABLED were defined)

@@ -296,7 +297,7 @@ return BLOSC_HAVE_NOTHING;
static shuffle_implementation_t get_shuffle_implementation(void) {
blosc_cpu_features cpu_features = blosc_get_cpu_features();
#if defined(SHUFFLE_AVX512_ENABLED)
if (cpu_features & BLOSC_HAVE_AVX512) {
if (cpu_features & BLOSC_HAVE_AVX512 && is_shuffle_avx2 && is_bshuf_AVX512) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Runtime check of the const bool defined by each SIMD implementation

@mgorny mgorny merged commit ce42731 into mgorny:shuffle-flags Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants