[FFmpeg-devel] [PATCH] Optimization of original IFF codec
Fri Apr 23 11:34:35 CEST 2010
M?ns Rullg?rd a ?crit :
> That one in particular is easier to read before your patch. If the
> compiler does the right thing with our default settings, leave it.
Although we discussed this yesterday in #ffmpeg-devel I'm replying to
this, so others here who didn't read the log at least know what's going on.
We already found out that the compiler won't replace the / with >> since
signed division differs from bit-shift right. We discussed that I shall
change this stuff to unsigned then.
I'll do a patch for this soon, but I want to mention though, that the
code in IFF does fiddle with bit-planar modes, thus the data is expected
to be calculated bit-wise. In that case I find it more readable to use
<< and >> instead of * and /.
Amiga bit-planar modes differ to chunky in that each pixel is a set bit.
Plane 0 is bit 0, plane 1 is bit 1, etc.
By OR'ing these bits together you get the color index.
Hope this clears things up, that's why I consider it more readable in
that case to also use bit shifting instead mul/div.
To summarize up, for readibility, I would recommend:
Use mul/div for math stuff which should be considered decimal, use <<
and >> for math stuff which should be considered binary (bit-wise).
> That's not how the optimiser works. If the optimiser runs at all,
> that transformation takes no extra time.
Consider this code in the optimizer:
if ( ASM_INSTRUCTION == X86_DIV && is_log2(dividend) )
replace_asm ( X86_SHR );
replace_dividend ( log2(dividend );
If the instruction is already shift then the block inside if isn't
executed at all, that what's I meant with the executed code differs.
> The same code runs in both cases.
Yes, but not the same if-conditions yield (see example above), unless
optimizers don't act this way.
>> 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.
It looks that you misunderstood me here a bit, imagine the following:
[...] some hundred lines of code, then comes a:
a / 2 (yes I know you should write 2.0 then, but you know...)
If you don't remind if a was declared as float or int (sometimes it's
even declared in a different file, the header file), it's unclear here
if it's a float div or int div.
But, if you write:
a >> 1
It's absolutely clear that a MUST be an int...hope this is expresses a
better way I wanted to mention.
> Hopefully you learned something about the optimiser. :-)
Yes, I did indeed. ;-)
:-) Basty/CDGS (-:
More information about the ffmpeg-devel