[Ffmpeg-devel] [PATCH] protect cmov asm sections with HAVE_CMOV
Fri Oct 20 15:21:57 CEST 2006
Diego Biurrun wrote:
> On Fri, Oct 20, 2006 at 10:26:45AM +0200, Guillaume Poirier wrote:
>>Diego Biurrun wrote:
>>>On Fri, Oct 20, 2006 at 09:49:24AM +0200, Guillaume POIRIER wrote:
>>>Patch looks fine except for
>>>>--- libavcodec/cabac.h (revision 6742)
>>>>+++ libavcodec/cabac.h (working copy)
>>>>@@ -32,7 +32,9 @@
>>>>#define CABAC_BITS 16
>>>>#define CABAC_MASK ((1<<CABAC_BITS)-1)
>>>>#define BRANCHLESS_CABAC_DECODER 1
>>>>#define CMOV_IS_FAST 1
>>>>//#define ARCH_X86_DISABLED 1
>>>>@@ -454,7 +456,7 @@
>>>>-#if (defined CMOV_IS_FAST && __CPU__ >= 686)
>>>>+#if defined CMOV_IS_FAST
>>>This is rather silly IMO, I'd say just replace it by HAVE_CMOV.
>>>Are there benchmarks proving that this really is slower on some
>>>processors? Then configue should deal with this as well.
>>Yes, the benchmarks I'm run a few weeks ago showed that Netburst-based
>>processors (P4/P-D) have a pretty slow implementation of CMOV.
>>I'll run some new benchmarks with latest optimizations of CABAC, but
>>I'm pretty confident that they will only confirm that on them, CMOV is
>>too slow to be beneficial.
After reading a few tech docs, and some micro benchs, I can state out
loud that CMOV isn't fast on Netburst-based processors.
Since we need to both know if a cpu supports CMOV and if it's fast,
with attached patch, configure (which attached patch) will set both
> OK. My point was just that currently CMOV_IS_FAST is defined when
> HAVE_CMOV is defined, so they are equal. This should be done in
> configure. At least add a FIXME note so this is not forgotten.
Yes, I agree. I mainly wanted to split patches to the minimum, and let
configure define CMOV_IS_FAST in an upcoming patch.
Attached patch does just was you're asking (hopefully).
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
More information about the ffmpeg-devel