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

[Reimar Döffinger]

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.