[FFmpeg-devel] Patch: Inline asm fixes for Intel compiler on Windows

[Michael Niedermayer]

On Tue, Mar 18, 2014 at 04:52:55PM +1100, Matt Oliver wrote:
> > i think its better not to have unneeded code in there, so if its
> > needed it should stay, if not it should be removed
> 
> 
> Removed.
> 
> These constraints are there because of limitions/shortcommings of
> > intels asm() support on windows.
> > IMO we should keep such workarounds to the minimum possible
> 
> 
> Fair enough, I updated the patches accordingly. These are basically the
> minimum workarounds possible. The main changes occurred in yuv2rgb_template
> where all references were added to the YUV2RGB_OPERANDS macro.  This added
> 5 constraints (although in many situations only 1 is actually used). The
> one thing ill point out is that these additional named constraints are
> variables that are not actually from the YUV2RGB macro (which is why they
> weren't initially put in there) so this may make the code a little less
> readable for other devs as all these variables are used by the other
> functions that use the YUV2RGB macro not from within the macro itself.
> 
> Also I had to add an #if to define RGB_PACK24_B_OPERANDS as these variables
> are only defined if COMPILE_TEMPLATE_MMXEXT so having them all the time
> will result in errors. This again may be a little unclear for other
> developers as the variables in question (mask1101 etc.) havnt even been
> declared at this point in code. And of course the variables are not
> relevant to YUV2RGB macro and are only used in a couple of the later
> functions as they are only relevant to the RGB_PACK24_B macro and the
> functions that use it. Trying to separate the RGB_PACK24_B variables cant
> be simply done, in fact the simplest way to separate them is pretty much
> the way the original patch was done. So its up to you which one you prefer.

>  configure               |    3 +++
>  libavcodec/x86/mlpdsp.c |    4 ++--
>  2 files changed, 5 insertions(+), 2 deletions(-)
> b89e0bced3342badd213b5dccc454a829cc37658  0002-2-6-Check-for-nonlocal-inline-asm-labels.patch
> From 5674f153f6e45cf487907bbe67b7109f5d7a83c0 Mon Sep 17 00:00:00 2001
> From: Matt Oliver <protogonoi at gmail.com>
> Date: Tue, 18 Mar 2014 15:29:14 +1100
> Subject: [PATCH 2/6] [2/6] Check for nonlocal inline asm labels.

applied


[...]

>  configure                          |    3 +++
>  libavcodec/x86/cabac.h             |    3 ++-
>  libavcodec/x86/cavsdsp.c           |    2 ++
>  libavcodec/x86/dsputil_mmx.c       |    1 +
>  libavcodec/x86/h264_i386.h         |    2 ++
>  libavcodec/x86/idct_sse2_xvid.c    |    2 +-
>  libavcodec/x86/lpc.c               |    3 +++
>  libavcodec/x86/motion_est.c        |    7 ++++---
>  libavcodec/x86/simple_idct.c       |    1 +
>  libavcodec/x86/vc1dsp_mmx.c        |    6 ++++++
>  libavutil/x86/asm.h                |   36 +++++++++++++++++++++++++++++++++++-
>  libpostproc/postprocess_template.c |    7 +++++++
>  libswresample/x86/resample_mmx.h   |    2 ++
>  libswscale/x86/rgb2rgb_template.c  |   11 +++++++++++
>  libswscale/x86/swscale_template.c  |   12 ++++++++++++
>  libswscale/x86/yuv2rgb_template.c  |    9 +++++++++
>  16 files changed, 101 insertions(+), 6 deletions(-)
> 74d49e1ba28eff2023cc9738d5db65796c179c39  0003-3-6-Check-for-inline-asm-direct-symbol-reference.patch
> From 1f3868943e761d1826f4a56e6ec9951273d47d40 Mon Sep 17 00:00:00 2001
> From: Matt Oliver <protogonoi at gmail.com>
> Date: Tue, 18 Mar 2014 15:53:26 +1100
> Subject: [PATCH 3/6] [3/6] Check for inline asm direct symbol reference.

applied most of it


[...]
>  cabac.h      |    9 +++++++--
>  h264_i386.h  |    4 ++--
>  vp56_arith.h |    8 +++++++-
>  3 files changed, 16 insertions(+), 5 deletions(-)
> 389c1c1629a1f08bb7b8b4d2b85ad6335ecf9202  0004-4-6-Fix-for-broken-register-allocation-issues-with-I.patch
> From 8450aea3c8bbfabea218d9b1980eebe4bda24643 Mon Sep 17 00:00:00 2001
> From: Matt Oliver <protogonoi at gmail.com>
> Date: Tue, 18 Mar 2014 15:54:27 +1100
> Subject: [PATCH 4/6] [4/6] Fix for broken register allocation issues with
>  Intel.
> 
> ---
>  libavcodec/x86/cabac.h      | 9 +++++++--
>  libavcodec/x86/h264_i386.h  | 4 ++--
>  libavcodec/x86/vp56_arith.h | 8 +++++++-
>  3 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/x86/cabac.h b/libavcodec/x86/cabac.h
> index 0a68b7b..efcb192 100644
> --- a/libavcodec/x86/cabac.h
> +++ b/libavcodec/x86/cabac.h
> @@ -33,6 +33,11 @@
>  #else
>  #       define BROKEN_COMPILER 0
>  #endif
> +#if   (defined(__INTEL_COMPILER) && defined(_MSC_VER))
> +#       define BROKEN_REGISTER_ALLOCATION 1
> +#else
> +#       define BROKEN_REGISTER_ALLOCATION 0
> +#endif

i assume this is because the code gets miscompiled ?
did you report the miscompilation to intel ?

[...]

>  mpegvideoenc_template.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 2b12c0ec5941fd82fe2c38c5646db8c6583ad574  0005-5-6-Fixed-64bit-conformance-with-mvzbl.patch
> From 754b9dc487260dedfae4be38c88dc60734561f67 Mon Sep 17 00:00:00 2001
> From: Matt Oliver <protogonoi at gmail.com>
> Date: Sun, 9 Feb 2014 17:10:12 +1100
> Subject: [PATCH 5/6] [5/6] Fixed 64bit conformance with mvzbl.

this has aready been applied

also feel free to send a patch that adds yourself to MAINTAINERs
for all these intel compiler workarounds

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140319/cdc9b05e/attachment.asc>