libsndfile / libsamplerate

An audio Sample Rate Conversion library
http://libsndfile.github.io/libsamplerate/
BSD 2-Clause "Simplified" License
600 stars 167 forks source link

fail to build with Xcode in universal binary on OSX #191

Closed otristan closed 1 year ago

otristan commented 1 year ago

libsamplerate failed to build on xcode with arm64 x86_64 architecture

In order to fix the issue, you should add

if defined(i386) || defined(__x86_64__)

where #ifdef HAVE_IMMINTRIN_H is tested or don't rely on HAVE_IMMINTRIN_H

Thanks !

evpobr commented 1 year ago

Hi @otristan , thanks for report.

sezero commented 1 year ago

https://github.com/libsndfile/libsamplerate/pull/188 is the guilty party for that.

That patch was made solely because mingw x64 version used x87 asm instead of SSE2, and as it is, it is causing more problems than it solves -- here is a build error on i686-linux:

[  1%] Building C object src/CMakeFiles/samplerate.dir/samplerate.c.o
In file included from /home/sezero/libsamplerate/src/samplerate.c:19:
/home/sezero/libsamplerate/src/common.h: In function ‘psf_lrintf’:
/home/sezero/libsamplerate/src/common.h:176: warning: implicit declaration of function ‘_mm_cvtss_si32’
/home/sezero/libsamplerate/src/common.h:176: warning: implicit declaration of function ‘_mm_load_ss’
/home/sezero/libsamplerate/src/common.h: In function ‘psf_lrint’:
/home/sezero/libsamplerate/src/common.h:185: warning: implicit declaration of function ‘_mm_cvtsd_si32’
/home/sezero/libsamplerate/src/common.h:185: warning: implicit declaration of function ‘_mm_load_sd’
[  3%] Building C object src/CMakeFiles/samplerate.dir/src_linear.c.o
In file included from /home/sezero/libsamplerate/src/src_linear.c:19:
/home/sezero/libsamplerate/src/common.h: In function ‘psf_lrintf’:
/home/sezero/libsamplerate/src/common.h:176: warning: implicit declaration of function ‘_mm_cvtss_si32’
/home/sezero/libsamplerate/src/common.h:176: warning: implicit declaration of function ‘_mm_load_ss’
/home/sezero/libsamplerate/src/common.h: In function ‘psf_lrint’:
/home/sezero/libsamplerate/src/common.h:185: warning: implicit declaration of function ‘_mm_cvtsd_si32’
/home/sezero/libsamplerate/src/common.h:185: warning: implicit declaration of function ‘_mm_load_sd’
[  4%] Building C object src/CMakeFiles/samplerate.dir/src_sinc.c.o
In file included from /home/sezero/libsamplerate/src/src_sinc.c:19:
/home/sezero/libsamplerate/src/common.h: In function ‘psf_lrintf’:
/home/sezero/libsamplerate/src/common.h:176: warning: implicit declaration of function ‘_mm_cvtss_si32’
/home/sezero/libsamplerate/src/common.h:176: warning: implicit declaration of function ‘_mm_load_ss’
/home/sezero/libsamplerate/src/common.h: In function ‘psf_lrint’:
/home/sezero/libsamplerate/src/common.h:185: warning: implicit declaration of function ‘_mm_cvtsd_si32’
/home/sezero/libsamplerate/src/common.h:185: warning: implicit declaration of function ‘_mm_load_sd’
[  6%] Building C object src/CMakeFiles/samplerate.dir/src_zoh.c.o
In file included from /home/sezero/libsamplerate/src/src_zoh.c:19:
/home/sezero/libsamplerate/src/common.h: In function ‘psf_lrintf’:
/home/sezero/libsamplerate/src/common.h:176: warning: implicit declaration of function ‘_mm_cvtss_si32’
/home/sezero/libsamplerate/src/common.h:176: warning: implicit declaration of function ‘_mm_load_ss’
/home/sezero/libsamplerate/src/common.h: In function ‘psf_lrint’:
/home/sezero/libsamplerate/src/common.h:185: warning: implicit declaration of function ‘_mm_cvtsd_si32’
/home/sezero/libsamplerate/src/common.h:185: warning: implicit declaration of function ‘_mm_load_sd’
[  8%] Linking C shared library libsamplerate.so
[  8%] Built target samplerate
Scanning dependencies of target multi_channel_test
[  9%] Building C object tests/CMakeFiles/multi_channel_test.dir/multi_channel_test.c.o
[ 11%] Building C object tests/CMakeFiles/multi_channel_test.dir/calc_snr.c.o
[ 13%] Building C object tests/CMakeFiles/multi_channel_test.dir/util.c.o
[ 14%] Linking C executable multi_channel_test
../src/libsamplerate.so.0.2.2: undefined reference to `_mm_load_sd'
../src/libsamplerate.so.0.2.2: undefined reference to `_mm_load_ss'
../src/libsamplerate.so.0.2.2: undefined reference to `_mm_cvtss_si32'
../src/libsamplerate.so.0.2.2: undefined reference to `_mm_cvtsd_si32'
collect2: ld returned 1 exit status

That's only because immintrin.h is present, but __SSE2__ is not defined.

I suggest that PR #188 be reverted and something like the following should be added to common.h only to address the windows x64 issue -- I can create a pull request for it if so wanted:

/*----------------------------------------------------------
** SIMD optimized math functions.
*/

#if defined(_WIN32) && (defined(_M_X64) || defined(__x86_64__))
#include <immintrin.h>

#undef  lrint
#define lrint  sse2_lrint
static inline int sse2_lrintf (float x)
{
    return _mm_cvtss_si32 (_mm_load_ss (&x)) ;
}

#undef  lrintf
#define lrintf sse2_lrintf
static inline int sse2_lrint (double x)
{
    return _mm_cvtsd_si32 (_mm_load_sd (&x)) ;
}
#endif
evpobr commented 1 year ago

Hi @otristan , @sezero .

otristan commented 1 year ago

That would do.