On Sun, Sep 14, 2014 at 01:01:45AM +0200, Mikulas Patocka wrote: > > > Hi > > > > > > Here I'm sending two patches to fix portability for 586-class machines > > > (Pentium, K6, etc.) > > > > > > > > > If the CPU is generic, 386, 486 or pentium, we must not use cmov in inline > > > assembler. > > > > > > Note that some Linux distributions are compiled for i686, and for them it > > > is > > > possible to use cmov in the assembler (because gcc uses it anyway). To > > > test for > > > this case, we test for defined(__i686__) || defined(__athlon__) || > > > defined(__SSE__) (these macros are driven by -march flag to gcc) and use > > > cmov if > > > one of these conditions is true. > > > > > > --- > > > configure | 4 ++++ > > > libavcodec/x86/mathops.h | 2 +- > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > Index: ffmpeg/configure > > > =================================================================== > > > --- ffmpeg.orig/configure 2014-09-12 21:46:25.710512294 +0200 > > > +++ ffmpeg/configure 2014-09-12 21:46:32.099587711 +0200 > > > @@ -3840,8 +3840,12 @@ elif enabled sparc; then > > > elif enabled x86; then > > > > > > case $cpu in > > > + generic) > > > + disable i686 > > > + ;; > > i686 extensions were specifically enabled some time ago by default, > > since we live in a modern world. > > If you're building for a older system, its your responsibility to pass > > the appropriate option. > > > > > i[345]86|pentium) > > > cpuflags="-march=$cpu" > > > + disable i686 > > > disable mmx > > > ;; > > > # targets that do NOT support nopl and conditional mov (cmov) > > > Index: ffmpeg/libavcodec/x86/mathops.h > > > =================================================================== > > > --- ffmpeg.orig/libavcodec/x86/mathops.h 2014-09-12 > > > 21:46:05.823390224 +0200 > > > +++ ffmpeg/libavcodec/x86/mathops.h 2014-09-12 21:46:32.116251966 > > > +0200 > > > @@ -69,7 +69,7 @@ static av_always_inline av_const int64_t > > > > > > #endif /* ARCH_X86_32 */ > > > > > > -#if HAVE_I686 > > > +#if HAVE_I686 || defined(__i686__) || defined(__athlon__) || > > > defined(__SSE__) > > > /* median of 3 */ > > > #define mid_pred mid_pred > > > static inline av_const int mid_pred(int a, int b, int c) > > > > I don't like this part, configure should control if the code is used, > > and if the i686 extensions are not enabled in configure, then they > > should not be built here. >
> In my opinion you could remove HAVE_I686 and disable/enable i686 at all. > > The user sets CFLAGS="-march=xxx" variable when running ./configure and it > selects the instruction set that is being generated. This is problematic, it would work with gcc and clang i assume but we support many more compilers and i suspect it wont work for all. Though maybe that wouldnt matter as they probably dont do gcc inline asm either But it would also be a special case as all other "what to built" cpu extensions are controled by configure --enable/disable- flags also having to use -march to disable could be annoying for debuging as -march affects alot more > > In the program, you can easily determine what -march was passed to the > compiler and react to it - if one of __i686__, __athlon__, __SSE__ is > defined, you can use cmov, because the compiler is already generating > cmov. thats true, but maybe its better if you put this check in configure so the --enable/disable flags can be used to override it but iam not really opposed to this hunk either if others are ok with it [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Avoid a single point of failure, be that a person or equipment.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
