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

[Matt Oliver]

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

> Matt Oliver <protogonoi <at> gmail.com> writes:
>
> > > Isn't the following sufficient?
>
> (Any reason why you cut the following line?)
>
> > > check_inline_asm inline_asm_direct_symbol_refs '"mov test, %eax\n"'
>
> > Need to also be able to declare 'int test' as other
> > build chains may fail due to missing declaration.
>
> Looking at your patch 1 I find that very unlikely
> or do I miss something?
>
> [...]
>
> > > Could you add the compilation error you see that makes
> > > patch 5 necessary?
> >
> > The first is
>
> [...]
>
> Without looking at the patch again, this seems
> to imply that you may / should split the patch.
>
> > Swapping MMX and MMXEXT around is due to the following line:
> >
> > #if COMPILE_TEMPLATE_MMXEXT
> > DECLARE_ASM_CONST(8, int16_t, mask1101[4]) = {-1,-1, 0,-1};
> > etc.
> >
> > When MMX is done first then the compiler generates
> > a series of errors about missing the maskXXXX
> > variables. Specifically:
> > ..\libswscale\x86\yuv2rgb_template.c(321): error :
> > identifier "mask1101" is undefined
> > and many others.
> >
> > It is possible to fix this by renaming the #if to
> > instead do it "#if COMPILE_TEMPLATE_MMX"
>
> This sounds as if compilation with --disable-mmxext
> would still fail with your patch, am I correct?
>
> I know I am a very bad example myself but please consider
> to fix your quoting, I believe this mail from you shows
> how important this is.
>
> Carl Eugen
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

> Looking at your patch 1 I find that very unlikely
> or do I miss something?

The main thing I was trying to avoid is that when checking for direct
symbol references that the line '"mov test, %eax\n"' fails because direct
symbols are not supported and not because the compiler has no idea what
'test' is where it is from. The fact patch 1 is needed is because Intel
needs variables declared but clearly as it hasnt been needed previously
then the current compilers dont appear to have that problem. However I was
trying to be forward thinking as should Intel update there compiler with
direct symbol references then the check will pick it up even if Intel still
has the declaration issue. And of course there may be other compilers out
there that have similar issues so the thinking was to make sure it was done
right the first time and make it clear exactly what was being tested.

> This sounds as if compilation with --disable-mmxext
> would still fail with your patch, am I correct?

Correct. Nice pick up as that didnt occur to me. Id have to change it so
that it will test if they have already been declared (perhaps add an extra
define). The other option would be to move them to yuv2rgb.c but given
these variables are used in yuv2rgb_template.c it may be better to keep
them in there so theres no risk of the declarations getting mistakenly
orphaned by future code changes.