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

[Diego Biurrun]

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
> >>+#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.

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.

Diego