From freedesktop-xorg  Tue Jul 26 07:04:00 2005
From: Lars Knoll <lars () trolltech ! com>
Date: Tue, 26 Jul 2005 07:04:00 +0000
To: freedesktop-xorg
Subject: Re: CVS Xserver breaks on non-SSE capable i386 machines
Message-Id: <200507260904.00582.lars () trolltech ! com>
X-MARC-Message: https://marc.info/?l=freedesktop-xorg&m=117278283829733

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