[MPlayer-dev-eng] [PATCH] x86_64 mmx/sse/3dnow optimisation support

[Aurelien Jacobs]

On Sat, 16 Oct 2004 22:13:36 -0400
D Richard Felker III <dalias at aerifal.cx> wrote:

> On Sun, Oct 17, 2004 at 02:16:17AM +0200, Aurelien Jacobs wrote:
> > On Sat, 16 Oct 2004 20:19:50 +0200
> > Jan Killius <jkillius at arcor.de> wrote:
> > 
> > > Hello,
> > > the patch don't work on my machine.
> > > Compiler version: gcc version 3.4.3 20041015 (prerelease)
> > > 
> > > cc -c -I../libvo -I../../libvo -I/usr/X11R6/include -O4   -pipe
> > > -ffast-math -fomit-frame-pointer -D_REENTRANT -D_LARGEFILE_SOURCE
> > > -D_FILE_OFFSET_BITS=64  -Ilibmpdemux -Iloader -Ilibvo
> > > -I/usr/include/freetype2   -I/usr/include/SDL -D_REENTRANT
> > > -I/usr/X11R6/include     -I/usr/include/  -o cpudetect.o cpudetect.c
> > > {standard input}: Assembler messages:
> > > {standard input}:2492: Error: suffix or operands invalid for `pop'
> > > {standard input}:2495: Error: suffix or operands invalid for `push'
> > > {standard input}:2498: Error: suffix or operands invalid for `pop'
> > > make: *** [cpudetect.o] Error 1
> > 
> > Unfortunatly, I don't have a gcc 3.4.3 prerelease here, nor the
> > bandwidth to download it right now :-(
> > I can't repoduce this with gcc 3.4.1.
> > 
> > Anyway I've digged a little bit more in the cpudetect code, and
> > made a very small change. The code is now cleaner. It don't
> > make any difference on the gcc versions I have, but I hope it
> > can make gcc 3.4.3 happier.
> > Could you please test this new version ?
> 
> changes to pullup.c rejected. it's not meant to be mplayer-dependent.

Ok, didn't know that.

> just don't tell pullup that mmx/etc is supported if you're on x86-64
> and maybe sometime i'll come up with a better solution. (note that
> pullup.c did not already include cpudetect.h, for good reason!)

Ok.
I see 2 solutions :
 - I could replace the inclue of cpudetect.h by defines of REG_a, etc..
   which would still keep mmx supported on x86_64 without mplayer
   dependencie.
 - Else, I'll need to replace the #ifdef HAVE_MMX by a 
   #if defined(HAVE_MXX) && !defined(ARCH_X86_64)

Which one do you prefer ? (I guess the second, but keeping x86_64 mmx
support may be nice)

> otherwise, i see lots of unnecessary changes from int->long in vf_*.c.
> please comment on why their necessary. i agree x86-64 support is
> useful, but i don't like excessive cosmetic patches especially when
> they reduce readability excessively like this.

I don't like cosmetic too !
If I added so much int->long that's because they were needed.

Here is an exemple :
	    "movq (%0,%2,2), %%mm1\n\t"
....
-	    : "r"(as), "r"(bs), "m" (ones), "0"(a), "1"(b), "X"(*a), "X"(*b) \
+	    : "r"((long)as), "r"((long)bs), "m" (ones), "0"(a), "1"(b), "X"(*a),

Here, %0 is used for addressing, so it need to be a 64 bits register.
as and bs are ints, so they will expand to 32 bits register and this
will prevent compilation.


But indeed, in some cases such as this one :
-		: "S" (a), "D" (b), "a" (s)
+		: "S" (a), "D" (b), "a" ((long)s)

s is an int and is explicitely put in eax. Then in the code, REG_a
is used explicitely. In that particular case (explicitely named
register), the cast is not necessary. But setting a 32 bits register
will zero expand to 64 bits. As s is an int, I guess this might lead
to sign problems !

Aurel

Aurel