[FFmpeg-devel] [PATCH] stricter constraints for libavcodec/i386/mpegvideo_mmx.c

[Alexis Ballier]

Bah.. resending because the original mail got rejected due to the size
of attachments...  benchs are now bzipped, granted it's more annoying
to read :(

>> All these variables are used as left operands of a movd instruction,
>> which does accept only memory or register operands while the "g"
>> constraint also allows immediates. This patch shall reflect this by
>> using "mr" instead.
>
> I would prefer "rm" over "mr" besides

attached

> please provide START/STOP_TIMER benchmarks of each changed function

that's gonna be a lot of files, and I hope I did it right (moreover i
think they're not needed wrt the following -S output analysis but here
they are):

I have put the START/STOP_TIMER stuff at the beginning/end of each
function (to be sure that's the right one we're benchmarking)

FWIW, I've used:
http://samples.mplayerhq.hu/V-codecs/h264/nature_704x576_25Hz_1500kbits.h264
as input

command line used:
for h263:
./ffmpeg -benchmark -y -i nature_704x576_25Hz_1500kbits.h264 foo.mp4
for mpeg1:
./ffmpeg -benchmark -y -i nature_704x576_25Hz_1500kbits.h264 foo.mpg
for mpeg2:
./ffmpeg -benchmark -y -i nature_704x576_25Hz_1500kbits.h264 foo.vob

each file is a cat of 5 runs of stderr output

ftracer_rm_* are those with the rm constraints and -ftracer in cflags
i cant build it with ftracer with the "g" constraint, so no bench there
rm_* are those with stock cflags (ie only ./configure && make) with rm
constraints
g_* are those with g constraints and stock cflags


I dont notice any real difference between g_* and rm_*
Funny thing is that ftracer seems to slow down:
- mpeg1_intra
- mpeg2_intra
- h263_intra
be the same for:
- mpeg1_inter
- h263_inter
and speedup:
- mpeg2_inter

> the gcc output from the errors

its not gcc that gives errors, he is happy to compile the code, but it
breaks as' teeth by trying to feed it wrong asm:

gcc -DHAVE_AV_CONFIG_H -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -I.
-I"/home/alexis/ffmpeg/svn/ffmpeg-build" -O2 -ftracer -D_ISOC99_SOURCE
-D_POSIX_C_SOURCE=200112 -fasm -std=c99 -fomit-frame-pointer -g
-Wdeclaration-after-statement -Wall -Wno-switch
-Wdisabled-optimization -Wpointer-arith -Wredundant-decls
-Wno-pointer-sign -Wcast-qual -Wwrite-strings -Wtype-limits -O3
-fno-math-errno -fno-signed-zeros       -c -o
libavcodec/i386/mpegvideo_mmx.o libavcodec/i386/mpegvideo_mmx.c
libavcodec/i386/mpegvideo_mmx.c: Assembler messages:
libavcodec/i386/mpegvideo_mmx.c:61: Error: suffix or operands invalid for `movd'

note that this seems new in gcc 4.3(.1?) (4.2.4 doesnt put an
immediate here) and the only way i've seen it happenning is with
-ftracer, probably due to more agressive optimisations.

> and if possible some information of what, if anything changes in the
> asm output

With -ftracer, diffing gcc -S output, I see some register naming
changes (probably doesn't change anything, sometimes X increases in
%rX, sometimes it decreases and is probably due to different register
allocation policy due to the constraint changes); the important part
is:
 -       movd $0, %mm5
+       movd %r8, %mm5
(- is unpatched version, + patched with "rm")
then there are some label changes, some memory addresses changes in
the last sections and that's about it.

the asm output without ftracer is absolutely identical here.


> yes, the functions are rather important to performance and i dont want
> changes to, not even bugfixes withot understandig what effect these
> changes have to performance.

understandable


Alexis.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mpegvide_mmx_asm_constraintsbis.patch
Type: application/octet-stream
Size: 2481 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080823/3e89d511/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bench.tar.bz2
Type: application/x-bzip2
Size: 20865 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080823/3e89d511/attachment.bin>