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

[Matt Oliver]

On 29 December 2013 13:48, Michael Niedermayer <michaelni at gmx.at> wrote:

> On Sun, Dec 29, 2013 at 01:25:05AM +1100, Matt Oliver wrote:
> > On 29 December 2013 00:58, Reimar Döffinger <Reimar.Doeffinger at gmx.de
> >wrote:
> >
> > > On 28.12.2013, at 14:28, Michael Niedermayer <michaelni at gmx.at> wrote:
> > > > On Sat, Dec 28, 2013 at 11:35:31AM +1100, Matt Oliver wrote:
> > > >> Hey,
> > > >> I made a patch that changes some of the inline asm found in FFmpeg
> so
> > > that
> > > >> it will compile under windows natively using the Intel compiler. The
> > > Intel
> > > >> compiler supports AT&T inline assembly syntax but doesnt have all
> the
> > > >> functionality that GAS has. A result is that a few things need to be
> > > >> slightly tweaked. Mostly this just involves changing direct symbol
> > > >> references to asm-interfaces.
> > > >>
> > > >> The attached patch should not change the functionality of FFmpeg in
> any
> > > way
> > > >> as it just changes the syntax of some instructions so that it will
> > > compile.
> > > >>
> > > >> That said there are a few sections where the code required slightly
> more
> > > >> complicated changes in order to work. These are x86/cabac.h in the
> > > >> BRANCHLESS_GET_CABAC macro and mlpdsp.c. The macro in cabac.h just
> > > requires
> > > >> some slightly more complicated asm-interface conversions that
> someone
> > > more
> > > >> familiar with the code may want to have a look at. mlpdsp.c uses
> some
> > > >> runtime jump lists that would require an almost complete rewrite to
> > > work on
> > > >> Intel which is probably not worth it. So at the moment both of
> these are
> > > >> simply skipped if the Intel compile is detected. All other asm works
> > > fine
> > > >> though.
> > > >>
> > > >> Code was compiled on Windows in Intel and using gcc 4.8 to ensure
> > > >> compatibility. All code was tested and past FATE.
> > > >
> > > > The patch doesnt apply:
> > > >
> > > > Applying: Patch: Inline asm fixes for Intel compiler on Windows
> > > > Using index info to reconstruct a base tree...
> > > > error: patch failed: libavcodec/x86/mpegvideoenc_template.c:168
> > > > error: libavcodec/x86/mpegvideoenc_template.c: patch does not apply
> > > > Did you hand edit your patch?
> > > > It does not apply to blobs recorded in its index.
> > > > Cannot fall back to three-way merge.
> > > >
> > > >
> > > > trying to apply the patch with "patch" does not result in code that
> > > > can be compiled.
> > > > In file included from
> > > /home/michael/ffmpeg-git/ffmpeg/libswscale/x86/yuv2rgb.c:57:0:
> > > >
> /home/michael/ffmpeg-git/ffmpeg/libswscale/x86/yuv2rgb_template.c:333:9:
> > > error: memory input 7 is not directly addressable
> > > >
> /home/michael/ffmpeg-git/ffmpeg/libswscale/x86/yuv2rgb_template.c:333:9:
> > > error: memory input 8 is not directly addressable
> > > >
> /home/michael/ffmpeg-git/ffmpeg/libswscale/x86/yuv2rgb_template.c:333:9:
> > > error: memory input 9 is not directly addressable
> > > >
> /home/michael/ffmpeg-git/ffmpeg/libswscale/x86/yuv2rgb_template.c:333:9:
> > > error: memory input 10 is not directly addressable
> > > >
> /home/michael/ffmpeg-git/ffmpeg/libswscale/x86/yuv2rgb_template.c:333:9:
> > > error: memory input 11 is not directly addressable
> > > >
> > > >
> > > > about the changes themselfs, its unlikely that adding operands
> > > > to asm will work. These will break some versions of gcc and clang
> > > > on x86-32 (and i would not be surprised if they break intels
> > > > compiler the same way)
> > >
> > > More importantly, it actually changes the code.
> > > As soon as PIC is enabled, a "m" reference can no longer be stored as a
> > > constant.
> > > But the code requires it to be a constant.
> > > Or to put it simpler: the patch is simply wrong for 32 bit PIC.
> > > (and before we start a discussion again: yes, this "hack" means FFmpeg
> has
> > > TEXTRELs in the asm code even for PIC compilation. Since it affects
> only 32
> > > bit I don't consider it very relevant).
> > > That said, I also have a very hard time believing that ICC assembler
> > > support cannot access global symbols from asm.
> > > I find that highly unlikely, more like the MANGLE macro and/or the
> macros
> > > that force these symbols to be exported are not working.
> >
> >
> > OK Sorry about the patch not sure what happened there. Im downloading a
> > fresh copy of git master that ill use to make up a fresh one.
> >
> > As for Intels asm support, on Linux they just use GAS directly for
> > compiling all asm. But on Windows they had to implement it themselves and
> > so it is incomplete.
> > To quote Intel themselves "Direct symbol references in the asm-template
> are
> > not supported. To access a C++ object, use the asm-interface with a
> > substitution directive."
> > This is from :
> >
> http://software.intel.com/sites/products/documentation/doclib/iss/2013/compiler/cpp-lin/GUID-5100C4FC-BC2F-4E36-943A-120CFFFB4285.htm
> > which also lists all the other limitations of inline AT&T asm on Windows.
> >
> > So unfortunately there is not really any other option (that I can think
> of
> > anyway). So if using memory "m" constraint cant be used with PIC how
> would
> > you recommend this problem be approached?
> >
> > Im happy to allocate the time needed to come up with the ideal solution
> so
> > any input is appreciated.
>
> code that is not frequently called could be rewritten to yasm syntax
> and put in seperate .asm files, this will not work for all cases
> though, also it would be quite alot of work and would be best done
> in small steps and patches ...
>
> or maybe intels assembler could be fixed or replaced
>
> but first, theres still something i dont understand
> C does support accesing global and static symbols,
> am i assuming correctly that intels C compler generates asm that is
> mixed with the asm from asm() statements?
> If so what does intels C compiler generate to access such symbols ?
> Iam asking as the same could be used to implement MANGLE() possibly
>
> Thanks
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Freedom in capitalist society always remains about the same as it was in
> ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
>

YASM would be the obvious solution to ensure wide compiler compatibility
but as you said would require a significant amount of work. Also would
there be a notable performance decrease to doing this? Is inline still
preferred?

Intels compiler handles symbol naming pretty much as you would expect. The
problem is that the inline asm compiler seems to be separated in that the
generated code from the inline asm pass resolves symbol names before being
linked in with the rest of the code (or in this case errors on symbol
names). So symbols are not exported across units unless they are in the
asm-interface. So any attempt to use a direct symbol reference will
automatically generate an error such as:
"error : The direct symbol reference in mov is currently unsupported.
 Access symbols through the asm interface."

So it doesnt even bother to check the symbol name it just automatically
errors should anyone try to reference one directly.

The ideal solution would be Intel updating there compiler to support direct
symbols but I believe they have indicated it is not high on their list of
priorities and I doubt they will let someone else try and fix it (hell id
fix it if they'd let me!). So if someone knows someone on the compiler team
then feel free to prod them otherwise we still have to try and work around
it as best we can.

If using "m" is an issue with PIC then that posses a problem. I had hoped
the compiler would be smart enough to optimise constants but I guess that
is to much to ask. The only other option I can think of is to have
different versions. i.e. for those few lines with direct symbol references
have an #ifdef that only changes the code for Intel compiler on Windows.
Windows shared libraries dont need PIC so this shouldn't be a problem.
Although the code may become a bit ugly.

Anyway I have created a new patch based on current git master. It doesnt
include any new changes from last time but it should apply correctly. Ill
update the patch once theres a consensus on how to handle PIC etc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg-inline-asm-intel-windows.patch
Type: application/octet-stream
Size: 117073 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131229/e9881232/attachment.obj>