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

Zuxy Meng zuxy.meng at gmail.com
Mon May 28 03:57:14 CEST 2007


Hi,

2007/5/26, Zuxy Meng <zuxy.meng at gmail.com>:
> Hi,
>
> 2007/5/25, Zuxy Meng <zuxy.meng at gmail.com>:
> > Hi,
> >
> > 2007/5/25, Guillaume Poirier <gpoirier at mplayerhq.hu>:
> > > Hi,
> > >
> > > Reimar Döffinger wrote:
> > > > Hello,
> > > > On Fri, May 25, 2007 at 02:14:49PM +0800, Zuxy Meng wrote:
> > > >>  We've reached Part 5. This is again a big diff, replacing many 'long'
> > > >>  to 'int', and several 'real' to 'short'. 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, or the declaration
> > > >>  didn't match its use (declared as float, used as short).
> > > >
> > > > These are separate issues and should be fixed in different patches. Also
> > > > if you changed long to int because the code assumes 32 bits, it would
> > > > make more sense to change it to int32_t. Replacing long with int without
> > > > thinking can make the code even slower on 64 bit systems.
> > >
> > > [..]
> > >
> > > >
> > > > This is unrelated and also completely useless since a static function
> > > > that is used in only one place will be inlined anyway.
> > > > In addition it is wrong since "static void make_decode_tables()" !=
> > > > "static void make_decode_tables(void)"
> > >
> > >
> > > Thanks for your quality review Reimar! I realize I'm quite a lousy
> > > reviewer, but I'm trying to do my best! :-(
> >
> > But you're always a prompt tester and more importantly, a nice guy
> > who's easy to get along with:-)
> >
> > Anway I've updated the patch according to suggestions from you and
> > Reimar, which should be cleaner.
> >
> > In this patch, long->int conversions are carried out because any of
> > the following:
> > 1. Assumed 32-bit access either by inline assembly or by bswap_32
> > macros. (e.g. variables in decode_i586.c, costab_mmx in decode_MMX.c
> > and newhead in sr1.c)
> > 2. Value that never exceed 32 bits. (such as variables representing
> > sampling frequency and frame size, variables initialized to known
> > 32-bit integers, and variables that are assigned by an 'int' and are
> > used to pass an 'int' parameter.
>
> I've tested the patch on IA-32 and AMD64 (thanks to Stefan's testbed!)
> and no regression has been found.

Would u help review the new patch, Reimar?
-- 
Zuxy
Beauty is truth,
While truth is beauty.
PGP KeyID: E8555ED6



More information about the MPlayer-dev-eng mailing list