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

[Reimar Döffinger]

Hello,
On Fri, May 25, 2007 at 09:24:58PM +0800, Zuxy Meng wrote:
> 2007/5/25, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> > > -static void make_decode_tables(long scaleval)
> > > +static void make_decode_tables()
> > >  {
> > > -  int i,j,k,kr,divv;
> > > +  int i,j,k,kr,divv,scaleval=32768;
> >
> > This is unrelated and also completely useless since a static function
> > that is used in only one place will be inlined anyway.
> 
> What I feel uncomfortable is the file scope static variable
> 'outscale'. An immediate or even a constant local variable looks
> better to me. I had to admit it's more an aesthetic matter.

And still unrelated ;-). And changing the static variable into a define
and using that as parameter seems like a better solution to me than
removing the parameter.
Or if you remove the parameter, adjust the code to it. At least the
first scaleval = -scaleval; is then completely useless.
And if you decide to optimize that function, get rid of the useless
divisions (e.g. i % 32).

> > In addition it is wrong since "static void make_decode_tables()" !=
> > "static void make_decode_tables(void)"
> 
> Really? I know that () isn't equivalent to (void) in declarations but
> is (void) needed even in definitions?

Why should it be treated different depending on whether there is a
function body directly below it or not?

Greetings,
Reimar Döffinger