[Mesa-dev] [PATCH 1/2] configure.ac: roll up a program for the sse4.1 check

[Erik Faye-Lund]

On Mon, Nov 17, 2014 at 3:50 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Mon, Nov 17, 2014 at 9:44 AM, Erik Faye-Lund <kusmabite at gmail.com> wrote:
>> On Sat, Nov 15, 2014 at 7:18 PM, Matt Turner <mattst88 at gmail.com> wrote:
>>> On Sat, Nov 15, 2014 at 10:13 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>> On Sat, Nov 15, 2014 at 12:04 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>>> So when checking/building sse code we have three possibilities:
>>>>>  1 Old compiler, throws an error when using -msse*
>>>>>  2 New compiler, user disables sse* (-mno-sse*)
>>>>>  3 New compiler, user doesn't disable sse
>>>>>
>>>>> The original code, added code for #1 but not #2. Later on we patched
>>>>> around the lack of handling #2 by wrapping the code in __SSE4_1__.
>>>>> Yet it lead to a missing/undefined symbol in case of #1 or #2, which
>>>>> might cause an issue for #2 when using the i965 driver.
>>>>>
>>>>> A bit later we "fixed" the undefined symbol by using #1, rather than
>>>>> updating it to handle #2. With this commit we set things straight :)
>>>>>
>>>>> To top it all up, conventions state that in case of conflicting
>>>>> (-enable-foo -disable-foo) options, the latter one takes precedence.
>>>>> Thus we need to make sure to prepend -msse4.1 to CFLAGS in our test.
>>>>>
>>>>> Cc: Siavash Eliasi <siavashserver at gmail.com>
>>>>> Cc: Matt Turner <mattst88 at gmail.com>
>>>>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>>>>> ---
>>>>>
>>>>> Man this thing is _very_ messy.
>>>>> Matt from the last hunk it seems that pixman might need fixing. Should
>>>>> be bother with that, or let people have fun when they hit it :P
>>>>>
>>>>> -Emil
>>>>>
>>>>>  configure.ac | 14 +++++++++++++-
>>>>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/configure.ac b/configure.ac
>>>>> index 91e111b..9d1835e 100644
>>>>> --- a/configure.ac
>>>>> +++ b/configure.ac
>>>>> @@ -252,7 +252,19 @@ AC_SUBST([VISIBILITY_CXXFLAGS])
>>>>>  dnl
>>>>>  dnl Optional flags, check for compiler support
>>>>>  dnl
>>>>> -AX_CHECK_COMPILE_FLAG([-msse4.1], [SSE41_SUPPORTED=1], [SSE41_SUPPORTED=0])
>>>>> +save_CFLAGS="$CFLAGS"
>>>>> +CFLAGS="-msse4.1 $CFLAGS"
>>>>> +AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
>>>>> +#include <mmintrin.h>
>>>>> +#include <xmmintrin.h>
>>>>> +#include <emmintrin.h>
>>>>> +#include <smmintrin.h>
>>>>> +int main () {
>>>>> +    __m128i a = _mm_set1_epi32 (0), b = _mm_set1_epi32 (0), c;
>>>>> +    c = _mm_max_epu32(a, b);
>>>>> +    return 0;
>>>>
>>>> This seems complicated.
>>>>
>>>> (a) Just #include <immintrin.h>
>>>> (b) Is any of this even necessary? how about
>>>>
>>>> int main() { return !__SSE_4_1__; }
>>>
>>> Checking that you can actually using the intrinsics seens like a good
>>> plan. Pixman's configure.ac has been doing that for a long time. I'd
>>> rather copy that. It's not like we'd save much by not doing it.
>>
>> ...But then we cannot cross-compile with run-time SSE 4.1 support from
>> a machine without SSE 4.1 support, no?
>
> Why not? It's a compile-time test. Obviously if there's no compiler
> support, then you're SOL...

Right, my bad. Thanks for setting the record straight.