[MPlayer-dev-eng] [PATCH] Make mp3lib SIMD optimizations work on AMD64, Part 5

[Zuxy Meng]

Hi,

2007/5/25, Guillaume Poirier <gpoirier at mplayerhq.hu>:
> Hi,
>
> Zuxy Meng wrote:
> > Hi,
> >
> > 2007/5/20, Zuxy Meng <zuxy.meng at gmail.com>:
> >> As discussed with Guillaume on IRC, I'll split my previous big patch
> >> (Rewrite synth_1to1_MMX....) into several small parts for easier
> >> review.
>
> [..]
>
> >> Part 5 will correct data types, replacing 'long' with 'int' where
> >> necessary.
> >>
> >> The last patch will deal with Makefile and macros.
> >
> > We've reached Part 5. This is again a big diff, replacing many 'long'
> > to 'int',
>
> These look Ok, though this certainly deserves to be tested on x86,
> x86-64, and PPC (other archs welcome).
>
>
> > and several 'real' to 'short'.
>
> These look fishy. For sure, "real" should be replaced by a standard C
> type:
>
> a simple grep shows:
>
> dct64_altivec.c:#define real float
> dct64_sse.c:typedef float real;
> decode_i586.c:#define real float /* ugly - but only way */
> dct64_k7.c:#define real float /* ugly - but only way */
> dct64_k7.c:#define real float /* ugly - but only way */
>
> sr1.c:#define real float
> dct64_3dnow.c:#define real float /* ugly - but only way */
> dct36_3dnow.c:#define real float /* ugly - but only way */
> decode_MMX.c:#define real float /* ugly - but only way */
>
> mpg123.h:#define REAL_IS_FLOAT
> mpg123.h:#  define real float
> mpg123.h:#  define real long double
> mpg123.h:#  define real double
>  ---> these aren't used, or are commented out.
>
> IMHO, this barbarism should be dealt with in a separate patch that
> simply s/real/float/g because as far as I can seen this is a leftover
> of an old version of the library.

I agree.
>
>
> > Among those, many are
> > necessary because under LP64 environment, sizeof(long) != sizeof(int)
> > and sizeof(long) != sizeof(float); others are optimizations simply
> > because we really don't need such a long type,
>
> These look OK, but need to be tested on all major archs, as I wrote above.

I've tested on IA-32 and am waiting for your results in AMD64:-)

> > or the declaration
> > didn't match its use (declared as float, used as short).
>
> This should be dealt with later on IMHO; after all, are they really
> necessary to add support for SIMD optimizations on AMD64? I'd be
> tempted to say no. Also, if you are going down this road, the other
> SIMD optimizations should be modified in a similar way, i.e. Altivec
> optimizations should be "fixed" too.

Altivec version doesn't have this problem. Anyway I'll split the patch
as you suggested.
-- 
Zuxy
Beauty is truth,
While truth is beauty.
PGP KeyID: E8555ED6