CVS Xserver breaks on non-SSE capable i386 machines

[Lars Knoll]

Hi,

On Sunday 24 July 2005 01:47, Krzysztof Halasa wrote:
> Unless I miss something, this change inconditionally adds "-msse" on
> all i386 machines with gcc 3.4+.
>
>
> The problems are:
>
> 1. "-msse" argument to gcc causes it to generate code using "cmov"
> instruction. Pentium MMX and earlier, and certain VIA C3 CPUs, lack
> cmov instruction, causing Xserver to die with SIGILL.

This is a problem. I didn't anticipate this as the gcc docs state that the 
flag only enables the use of the mmx/sse intrinsics and doesn't affect code 
generation. Obviously the docs are wrong :(

> Fix: Removing "-msse" for those files (fbmmx.c, fbpict.c, fbfill.c,
> fbcopy.c) should fix this problem.
>
>
>
> 2. fbmmx.c does:
> #include <xmmintrin.h> /* for _mm_shuffle_pi16 and _MM_SHUFFLE */
>
> xmmintrin.h (a gcc header file) can't be included unless "-msse" or
> equivalent is passed to gcc:
>
> #ifndef __SSE__
> # error "SSE instruction set not enabled"
>
> fbmmx.c uses _mm_shuffle_pi16 which is pshufw (SSE and not just MMX)
> instruction. fbmmx.c and friends use it when MMX is available (#defined
> USE_MMX and fbHaveMMX()) but it can't work on Pentium MMX and other
> non-SSE capable CPUs.

Yes, this needs SSE or the AMD MMX extensions. The basic question is if we 
want to support assembler code paths on CPUs older than PIII or Athlon.

Not using pshufw on newer CPUs is somewhat stupid (especially as older CPUs 
are getting rare). A compile time option is not good, as you wouldn't get a 
portable binary. So either we do not use the mmx code paths for older CPUs 
(that's what the current code is supposed to do), or we duplicate all code in 
fbmmx.c (something I didn't want to do).

> Fix: xmmintrin.h and "-msse" must not be used, pshufw (if available)
> must be called using other means (asm?) and a replacement routine must
> be written in case no SSE CPU is detected.
>
> Comments?
>
> I can try to fix it if the above is correct.

A simple solution is to remove the -mmmx and -msse flags from all but fbmmx.c, 
but keep -DUSE_MMX for the other files. As a second thing we will have to 
move the CPU detection into a file that is not compiled with -mmmx/-msse.

The attached patch would do exactly that.

Cheers,
Lars
-------------- next part --------------
A non-text attachment was scrubbed...
Name: detection.diff
Type: text/x-diff
Size: 7757 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg/attachments/20050725/b9fa5f6f/attachment.diff>