[FFmpeg-devel] [PATCH] Optimization of original IFF codec

[Måns Rullgård]

Sebastian Vater <cdgs.basty at googlemail.com> writes:

> Hi Mans!
>
> M?ns Rullg?rd a ?crit :
>>> But this time I'm not only replacing * with << and / with >> , but also
>>> were able  to remove a multiplication inside a loop replacing with an
>>> addition.
>>>     
>>
>> Did you check if these actually change the compiled code?  I bet most
>> of them don't.
>>   
> Unless it's not -O0 it probably won't change anything (the mul by add
> replacement will probably only optimized, if at all, at -O2 ...

That one in particular is easier to read before your patch.  If the
compiler does the right thing with our default settings, leave it.

> Also it's pretty meaningless just to compare the compiled code with just
> my compiler version, since ffmpeg should work with GCC 2 onwards...

These are trivial, low-level optimisations.  If one compiler does the,
chances are most of them do.

> But as said, we should be consequent on this, having here a * 2 and
> there a << 1 looks somewhat weird...it probably will also some small
> amounts of compile time, since the optimizer hasn't to do stuff like
> this anymore by itself.

That's not how the optimiser works.  If the optimiser runs at all,
that transformation takes no extra time.

> Also running less code in the compiler's optimizer code also reduces the
> change by triggering bugs in this code.

The same code runs in both cases.

> Of course, this is for each single a minimal save, but if there are over
> 1000th's of such changes of this style by hand, there might be saved one
> or other second of compile time.

I don't think so.

> It also forces people reading our code to think more bit-wise than
> decimal wise, which is pretty useful by the stuff here we're doing,
> since we're throwing a lot of &, |, ^, <<, >> around here, right? ;)

We have two priorities here:

1.  Fast code.
2.  Readable code.

In this case, speed is the same, since the same machine code is
generated.  That leaves readability as a concern.

> It also makes absolutely clear that we're dealing with integer
> arithmetic...a / 2 can either mean divide a float by int, or an int by int.

No.  / can only mean integer division when operating on integers.
What to watch out for here is signed vs unsigned division and shift.
Signed division (of negative values) is not equivalent to a shift.

> So consider this also as some kind of educational patch :D

Hopefully you learned something about the optimiser. :-)

-- 
M?ns Rullg?rd
mans at mansr.com