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

[Guillaume Poirier]

Hi,

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

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
defines.


> 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).

Guillaume
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: use_HAVE_CMOV_define.2.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20061020/78054d8b/attachment.txt>