CVS Xserver breaks on non-SSE capable i386 machines

[Xavier Bachelot]

Hi Lars,

Lars Knoll wrote:
> On Monday 25 July 2005 23:06, Nicholas Miell wrote:
> 
>>On Mon, 25 Jul 2005 12:43:25 +0200, Lars Knoll wrote:
>>
>>>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.
>>
>>Some commentary below.
>>
>>
>>>===================================================================
>>>RCS file: /cvs/xorg/xc/programs/Xserver/fb/Imakefile,v
>>>retrieving revision 1.10
>>>diff -u -p -r1.10 Imakefile
>>>--- Imakefile	12 Jul 2005 10:02:10 -0000	1.10
>>>+++ Imakefile	25 Jul 2005 10:42:05 -0000
>>>@@ -6,12 +6,12 @@ XCOMM Id: Imakefile,v 1.1 1999/11/02 03:
>>> #if defined(HasGcc34) && HasGcc34
>>> MMXOPTIONS= -mmmx -msse -Winline --param inline-unit-growth=10000 \
>>> 	--param large-function-growth=10000 -DUSE_MMX
>>>-
>>>+USEMMXOPTIONS= -DUSE_MMX -m32
>>
>>-m32 will break the build on AMD64.
> 
> 
> Yes, an oversight in my patch, as I'm using an AMD64 and had to test it does 
> the right thing in 32bit mode.
> 
> 
>>> #if defined(i386Architecture) || defined(AMD64Architecture)
>>> SpecialCObjectRule(fbmmx,fbmmx.c,$(MMXOPTIONS))
>>>-SpecialCObjectRule(fbpict,fbpict.c,$(MMXOPTIONS))
>>>-SpecialCObjectRule(fbfill,fbfill.c,$(MMXOPTIONS))
>>>-SpecialCObjectRule(fbcopy,fbcopy.c,$(MMXOPTIONS))
>>>+SpecialCObjectRule(fbpict,fbpict.c,$(USEMMXOPTIONS))
>>>+SpecialCObjectRule(fbfill,fbfill.c,$(USEMMXOPTIONS))
>>>+SpecialCObjectRule(fbcopy,fbcopy.c,$(USEMMXOPTIONS))
>>> #endif
>>>
>>> #endif
>>>
>>>===================================================================
>>>RCS file: /cvs/xorg/xc/programs/Xserver/fb/fbpict.c,v
>>>retrieving revision 1.16
>>>diff -u -p -r1.16 fbpict.c
>>>--- fbpict.c	12 Jul 2005 10:02:10 -0000	1.16
>>>+++ fbpict.c	25 Jul 2005 10:42:06 -0000
>>>@@ -1332,3 +1332,113 @@ fbPictureInit (ScreenPtr pScreen, PictFo
>>>
>>>     return TRUE;
>>> }
>>>+
>>>+
>>>+#ifdef USE_MMX
>>>+/* The CPU detection code needs to be in a file not compiled with
>>>+ * "-mmmx -msse", as gcc would generate CMOV instructions otherwise
>>>+ * that would lead to SIGILL instructions on old CPUs that don't have
>>>+ * it.
>>>+ */
>>>+#if !defined(__amd64__) && !defined(__x86_64__)
>>>+
>>>+enum CPUFeatures {
>>>+    NoFeatures = 0,
>>>+    MMX = 0x1,
>>>+    MMX_Extensions = 0x2,
>>>+    SSE = 0x6,
>>
>>Shouldn't SSE = 0x4? or SSE = (MMX_Extensions | 0x4)?
> 
> 
> The second. MMX_Extensions is for this purpose a subset of SSE, that's why I 
> did it this way. I make the code a bit clearer.
> 
> Lars

I'm still having this bug in 6.8.99.902 on some via CPU's (Ezra/Samuel). 
The patch doesn't apply anymore to RC2. Is there any chance it can be 
updated and committed to CVS ? I'm gonna try to update it on my own 
meanwhile, but it would be very nice to have it fixed before 6.9/7.0 
final :-)

Regards,

Xavier