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

[Matt Oliver]

On 9 February 2014 18:43, Carl Eugen Hoyos <cehoyos at ag.or.at> wrote:

> Matt Oliver <protogonoi <at> gmail.com> writes:
>
> > Patch 1: add missing external declarations (unmodified from
> > previous)
>
> > + extern uint8_t ff_h264_cabac_tables[512 + 4*2*64 + 4*64 + 63];
>
> Is including cabac_functions.h an alternative?
>
> > //Declared in swscale_mmx.c (and referenced from rgb2rgb_template.c)
>
> s/swscale_mmx.c/swscale.c
>
> > Patch 3: Asm direct symbol reference check and support
> > (rebased with minor addition to yuv2rgb_template.c that
> > fixes previous Patch 5 issue)
>
> > +check_cc <<EOF && enable inline_asm_direct_symbol_refs
> > +void foo(void){ int test; __asm__ volatile("movl test, %eax"); }
> > +EOF
>
> Please simplify as discussed.
>
> > Patch 4: Workaround for "register allocation failed" errors
> > found in intel compiler  (unmodified from previous)
>
> +#if   (defined(__INTEL_COMPILER) && defined(_MSC_VER))
> +#       define BROKEN_REGISTER_ALLOCATION 1
> +#else
> +#       define BROKEN_REGISTER_ALLOCATION 0
> +#endif
>
> +#if   (defined(__INTEL_COMPILER) && defined(_MSC_VER) && ARCH_X86_64)
> +#       define BROKEN_REGISTER_ALLOCATION 1
> +#else
> +#       define BROKEN_REGISTER_ALLOCATION 0
> +#endif
>
> I still wonder why this has to be duplicated.
>
> > Patch 5: Fixed 64bit conformance with movzbl. Fixes
> > "Unsupported instruction form in asm instruction movzbl"
> > found in intel compiler.
>
> (I probably just don't understand this patch:)
>
> > -        "movzb %%al, %%"REG_a"          \n\t" // last_non_zero_p1
> > +        "movzbl %%al, %%eax             \n\t" // last_non_zero_p1
>
> Why isn't the new line also using '%%"REG_a"'?
>
> Carl Eugen
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

> Is including cabac_functions.h an alternative?

Possibly, But I figured just adding a couple of externs instead of an
entire header keeps things a bit simpler for the compiler.

> s/swscale_mmx.c/swscale.c

Sorry I dont understand what your comment is referring to here. Do you want
me to remove that comment line from the patch?

> Please simplify as discussed.

OK, as i said I was trying to be forward thinking in avoiding future issues
but if youd prefer it simplified then I will do it that way.

> I still wonder why this has to be duplicated.

Because both generate an error due to register allocation but one only
generates an error in 64bit mode whereas the other generates an error in
both 32bit/64bit. However your right in that the second 64bit only error is
a slightly different register error than the first so I will give it a
separate define name to illustrate the slight differences better.

> Why isn't the new line also using '%%"REG_a"'?

When compiling in 64bit REG_a is set to RAX. movzbl is a 32bit instruction
that works on the 32bit EAX register. So you have to explicitly state eax
even in 64bit compilation otherwise you get an "Unsupported instruction
form in asm instruction movzbl" error. GCC is smart enough to handle the
conversion for you but intel is very strict.