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

Swapped out use of sincos() for more portability across platforms #11

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

dmtrinh
Copy link
Contributor

@dmtrinh dmtrinh commented Feb 15, 2024

sincos() is a GNU extension provided by glibc. With Xcode 15.1 on Apple Silicon, we get the following error:

gcc -Wall -Wextra -Werror -O3 -march=native -o bench_fftw main.c -lfftw3 -lm
main.c:49:9: error: call to undeclared function 'sincos'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
        sincos(angles[i], &sin_a, &cos_a);
        ^
main.c:49:9: note: did you mean '__sincos'?
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/math.h:658:29: note: '__sincos' declared here
__header_always_inline void __sincos(double __x, double *__sinp, double *__cosp) {

We can, technically, use preprocessor directives to get around this; for example:

        #ifdef __APPLE__
            __sincos(angles[i], &sin_a, &cos_a);
        #else
            sincos(angles[i], &sin_a, &cos_a);
        #endif

However, I feel that's not really elegant. To improve portability across platforms, we should switch to invoking sin() and cos() separately. Yes, this will be theoretically slower -- BUT many good compilers can detect this situation and perform optimization as necessary:
Compiler optimizations

@smu160 smu160 merged commit 25ce43a into QuState:main Feb 16, 2024
4 checks passed
@smu160 smu160 added the bug Something isn't working label Feb 16, 2024
@smu160 smu160 self-requested a review February 16, 2024 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants