[Ffmpeg-devel] [PATCH] protect cmov asm sections with HAVE_CMOV

[Guillaume Poirier]

Hi,

Diego Biurrun wrote:
> On Fri, Oct 20, 2006 at 09:49:24AM +0200, Guillaume POIRIER wrote:
> 
>>Now that configure script is able to "detect" if CMOV is supported, I
>>guess it's time to use it where appropriate, which is just what this
>>patch is supposed to do.
>>
>>Note that with this patch, CMOV_IS_FAST == HAVE_CMOV, but since we
>>(can) know (after having done some benchmark) on which CPU CMOV is
>>actually fast (pretty much all of them except netburst processors), we
>>can make configure set CMOV_IS_FAST if HAVE_CMOV==1 && !CMPU=P4 or
>>smth like that.
>>Thoughts?
> 
> 
> 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
>>+#ifdef HAVE_CMOV
>> #define CMOV_IS_FAST 1
>>+#endif
>> //#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.

Guillaume