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

[Aurelien Jacobs]

On Sun, 17 Oct 2004 11:30:19 -0400
D Richard Felker III <dalias at aerifal.cx> wrote:

> On Sun, Oct 17, 2004 at 04:41:28PM +0200, Aurelien Jacobs wrote:
> > > 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)
> 
> approximately the second. but put a separate #ifndef ARCH_X86_64 (or
> preferably #ifdef ARCH_X86, if it's not defined for x86-64) on the
> outside around the other #ifdef, since it's possible i'll add 3dnow or
> other optimized functions later.

Ok. Done.

> > > 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.
> 
> casting to long like this is fine. but there were lots of local
> variables where you changed the types.

I tried not to modify variable types too much, but in some cases it was
better, or even necessary. Most of the local types changes looks like
this :
-	int crap1, crap2;
+	long crap1, crap2;
...
 			: "=S"(crap1), "=D"(crap2)

So those variables are in fact the output values of some asm come.
The asm output 64 bits values, so it need a 64 bits variable for
storage. Anyway, gcc will complain if you try to use a cast as a
lvalue.

> there were also functions where
> you changed the types of function arguments which i find really bad.

Right. I also find this very bad !
In fact I tried to avoid this as much as possible. But when modifing
vf_ilpack.c, I found it would be far cleaner to modify parameters type,
and as those functions were statics and were used only once, I thought
it would not be a big problem.
At the end I used a totally different way to port those functions to
x86_64, but I forgot to check if the functions type modification was
still necessary. The good news is that it's not !
So I corrected this.
vf_ilpack.c was the only file where I modified some parameters type.

Aurel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: MPlayer-x86_64.diff.bz2
Type: application/octet-stream
Size: 18112 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20041017/46be09c3/attachment.obj>