GNOME Bugzilla – Bug 670690
audioresample: missing configure checks for SSE / SSE2
Last modified: 2014-01-20 15:11:34 UTC
Created attachment 208281 [details] [review] define _USE_SSE only if __SSE__ is defined Hi. Trying to build gstreamer-plugins-base om OpenBSD/i386 fails with the following error: cc -std=gnu99 -DHAVE_CONFIG_H -I. -I/usr/ports/pobj/gst-plugins-base-0.10.36/gst-plugins-base-0.10.36/gst/audioresample -I../.. -I/usr/local/include -I/usr/local/include/libpng -I/usr/X11R6/include -I/usr/p orts/pobj/gst-plugins-base-0.10.36/gst-plugins-base-0.10.36/gst-libs -I../../gst-libs -I/usr/local/include/gstreamer-0.10 -pthread -I/usr/local/include/glib-2.0 -I/usr/local/lib/glib-2.0/include -I/usr/loca l/include/libxml2 -I/usr/local/include -pthread -I/usr/local/include/glib-2.0 -I/usr/local/lib/glib-2.0/include -I/usr/local/include/gstreamer-0.10 -pthread -I/usr/local/include/glib-2.0 -I/usr/local/lib/gl ib-2.0/include -I/usr/local/include/libxml2 -I/usr/local/include -DG_THREADS_MANDATORY -DG_DISABLE_CAST_CHECKS -DG_DISABLE_ASSERT -Wall -Wdeclaration-after-statement -Wpointer-arith -Wmissing-declarations - Wmissing-prototypes -Wredundant-decls -Wundef -Wwrite-strings -Wformat-nonliteral -Wformat-security -Winit-self -Wmissing-include-dirs -Waddress -Waggregate-return -Wno-multichar -Wnested-externs -g -I/usr/ local/include/orc-0.4 -O2 -pipe -MT libgstaudioresample_la-speex_resampler_float.lo -MD -MP -MF .deps/libgstaudioresample_la-speex_resampler_float.Tpo -c /usr/ports/pobj/gst-plugins-base-0.10.36/gst-plugins -base-0.10.36/gst/audioresample/speex_resampler_float.c -fPIC -DPIC -o .libs/libgstaudioresample_la-speex_resampler_float.o In file included from /usr/local/include/glib-2.0/glib/gbacktrace.h:35, from /usr/local/include/glib-2.0/glib.h:36, from /usr/ports/pobj/gst-plugins-base-0.10.36/gst-plugins-base-0.10.36/gst/audioresample/resample.c:71, from /usr/ports/pobj/gst-plugins-base-0.10.36/gst-plugins-base-0.10.36/gst/audioresample/speex_resampler_float.c:26: /usr/include/signal.h: In function 'sigdelset': /usr/include/signal.h:82: warning: redundant redeclaration of '__errno' /usr/include/signal.h:71: warning: previous declaration of '__errno' was here /usr/include/signal.h: In function 'sigismember': /usr/include/signal.h:93: warning: redundant redeclaration of '__errno' /usr/include/signal.h:82: warning: previous declaration of '__errno' was here In file included from /usr/ports/pobj/gst-plugins-base-0.10.36/gst-plugins-base-0.10.36/gst/audioresample/resample.c:134, from /usr/ports/pobj/gst-plugins-base-0.10.36/gst-plugins-base-0.10.36/gst/audioresample/speex_resampler_float.c:26: /usr/ports/pobj/gst-plugins-base-0.10.36/gst-plugins-base-0.10.36/gst/audioresample/resample_sse.h: In function 'inner_product_single': /usr/ports/pobj/gst-plugins-base-0.10.36/gst-plugins-base-0.10.36/gst/audioresample/resample_sse.h:46: error: '__m128' undeclared (first use in this function) /usr/ports/pobj/gst-plugins-base-0.10.36/gst-plugins-base-0.10.36/gst/audioresample/resample_sse.h:46: error: (Each undeclared identifier is reported only once __m128 is declared in /usr/include/xmmintrin.h On OpenBSD, the file /usr/include/xmmintrin.h has the following: #ifndef __SSE__ # error "SSE instructions set not enabled" #else ... #endif This is not an issue on amd64 since __SSE__ and __SSE2__ are defined there, but not on i386: $ gcc -E -dM - < /dev/null | grep SSE $ Here's the relevant output of the different configure output on amd64 and i386: * amd64 checking for sys/wait.h... (cached) yes checking for sys/stat.h... (cached) yes checking xmmintrin.h usability... yes checking xmmintrin.h presence... yes checking for xmmintrin.h... yes checking emmintrin.h usability... yes checking emmintrin.h presence... yes checking for emmintrin.h... yes * i386 checking for sys/wait.h... (cached) yes checking for sys/stat.h... (cached) yes checking xmmintrin.h usability... no checking xmmintrin.h presence... no checking for xmmintrin.h... no checking emmintrin.h usability... yes checking emmintrin.h presence... yes checking for emmintrin.h... yes And the config.log on i386: configure:21882: checking xmmintrin.h usability configure:21882: cc -std=gnu99 -c -O2 -pipe -I/usr/local/include -I/usr/local/include/libpng -I/usr/X11R6/include conftest.c >&5 In file included from conftest.c:73: /usr/include/xmmintrin.h:35:3: error: #error "SSE instruction set not enabled" configure:21882: $? = 1 configure: failed program was: | /* confdefs.h */ We are currently using the attached patch but I think there should be a cleaner check (in configure or ...?). Thanks.
Is HAVE_XMMINTRIN_H or HAVE_EMMINTRIN_H defined in your config.h file (generated by configure)?
(In reply to comment #1) > Is HAVE_XMMINTRIN_H or HAVE_EMMINTRIN_H defined in your config.h file > (generated by configure)? Yes, HAVE_EMMINTRIN_H is. $ egrep '(HAVE_XMMINTRIN_H|HAVE_EMMINTRIN_H)' config.h #define HAVE_EMMINTRIN_H 1 /* #undef HAVE_XMMINTRIN_H */ Also as I mentioned on OpenBSD, the file "/usr/include/xmmintrin.h" looks like the following: #ifndef __SSE__ # error "SSE instructions set not enabled" #else ... #endif And on i386, __SSE__ is indeed not enabled.
Ok, then we should be able to use the HAVE_EMMINTRIN_H and HAVE_XMMINTRIN_H defines, right? They are already used in the code but apparently not correctly
I am not sure if this will work out, or maybe I didn't understand properly ;-) But on my system for e.g. HAVE_EMMINTRIN_H will be defined. However, since I don't have __SSE__ defined in cpp, including it will not provide the corresponding SSE related defines. I'm now investigating the reason we have this header file on i386 systems when it is not really usable.
Ok, so to summarize, I think SSE should be enabled in gstreamer only if: * HAVE_EMMINTRIN_H is defined * __SSE__ is defined Or something like that... i386: HAVE_EMMINTRIN_H is defined, but as we can see SSE is not, so the SSE code should not be used $ gcc -E -dM - < /dev/null | grep SSE $ amd64: HAVE_EMMINTRIN_H is defined and SEE as well, so all is good, we can compiled in the SSE parts $ gcc -E -dM - < /dev/null | grep SSE #define __SSE2_MATH__ 1 #define __SSE_MATH__ 1 #define __SSE__ 1 #define __SSE2__ 1 $
I have same issue on NetBSD-i386 as bug #671206 My proposed patch is here: --- gst/audioresample/resample.c.orig 2011-12-30 22:29:15.000000000 +0900 +++ gst/audioresample/resample.c 2012-03-02 21:27:45.000000000 +0900 @@ -77,13 +77,13 @@ #define EXPORT G_GNUC_INTERNAL #ifdef _USE_SSE -#ifndef HAVE_XMMINTRIN_H +#ifndef __SSE__ #undef _USE_SSE #endif #endif #ifdef _USE_SSE2 -#ifndef HAVE_EMMINTRIN_H +#ifndef __SSE2__ #undef _USE_SSE2 #endif #endif
*** Bug 671206 has been marked as a duplicate of this bug. ***
My point is that HAVE_XMMINTRIN_H shouldn't even be defined by configure if it causes an error to be included. Your patch is probably correct but IMHO there's something wrong with the configure check that causes this: checking xmmintrin.h usability... no checking xmmintrin.h presence... no checking for xmmintrin.h... no => This should never result in HAVE_XMMINTRIN_H being defined.
(In reply to comment #8) > My point is that HAVE_XMMINTRIN_H shouldn't even be defined by configure if it > causes an error to be included. Your patch is probably correct but IMHO there's > something wrong with the configure check that causes this: > > checking xmmintrin.h usability... no > checking xmmintrin.h presence... no > checking for xmmintrin.h... no > > => This should never result in HAVE_XMMINTRIN_H being defined. As far as I am concerned, on i386 HAVE_XMMINTRIN_H does not get defined which is correct.
AIUI the status of this bug hasn't changed. XMMINTRIN_H fails (correctly) to be defined on i386, but EMMINTRIN is defined, even though it's not usable without __SSE__ The simple patch to resample.c would seem to be sufficient.
Ok, could you provide the patch in "git format-patch" format? For this commit it locally (make sure to set your name and mail address in the git config) and then run "git format-patch -1".
(In reply to comment #11) > Ok, could you provide the patch in "git format-patch" format? For this commit > it locally (make sure to set your name and mail address in the git config) and > then run "git format-patch -1". Hi Sebastian. Since no one stepped up, here's a git-format patch. I hope this can be pushed soon. Thanks ;-)
Created attachment 266723 [details] [review] check for __SSE__ instead of HAVE_XMMINTRIN_H
Comment on attachment 266723 [details] [review] check for __SSE__ instead of HAVE_XMMINTRIN_H I think we need to check for HAVE_XXX and __SSE__ here. You might have SSE(2) but not the header.
Created attachment 266739 [details] [review] check for __SSE__ and __SSE2__ as well Hi sebastian. Is this what you had in mind?
commit daa194b71ea6f9e8ee522ab02e8c56150b7e62b3 Author: Antoine Jacoutot <ajacoutot@gnome.org> Date: Mon Jan 20 15:44:09 2014 +0100 audioresample: Fix build on x86 if emmintrin.h is available but can't be used On i386, EMMINTRIN is defined but not usable without SSE so check for __SSE__ and __SSE2__ as well. https://bugzilla.gnome.org/show_bug.cgi?id=670690
commit 4e3d101aa854cfee633a9689efeb75e5001baa5e Author: Sebastian Dröge <sebastian@centricular.com> Date: Mon Jan 20 16:11:04 2014 +0100 audioresample: It's HAVE_EMMINTRIN_H, not HAVE_XMMINTRIN_H for SSE2