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

[Guillaume Poirier]

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.


> 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.


> 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.

Guillaume

Guillaume