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

[Reimar Döffinger]

On 05.01.2014, at 03:27, Matt Oliver <protogonoi at gmail.com> wrote:

> On 30 December 2013 13:06, Matt Oliver <protogonoi at gmail.com> wrote:
> 
>> On 30 December 2013 02:26, Michael Niedermayer <michaelni at gmx.at> wrote:
>> 
>>> On Sun, Dec 29, 2013 at 02:19:40PM +0100, Reimar Döffinger wrote:
>>>> On Sun, Dec 29, 2013 at 02:48:52PM +1100, Matt Oliver wrote:
>>>>> 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.
>>>> 
>>>> One thing I thought of: You could try using named asm constraints, and
>>>> only change the MANGLE to convert to a reference to a named asm
>>>> argument.
>>>> If you are lucky in quite a few cases you'd then only need an ifdef
>>>> around the extra asm constraints.
>>> 
>>> that should be possible without ifdef, see XMM_CLOBBERS_ONLY() for
>>> something similar
>>> 
>>> [...]
>>> --
>>> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>> 
>>> You can kill me, but you cannot change the truth.
>>> 
>> 
>> Using named asm constraints is a good idea as it seems that Intel compiler
>> supports those. So I will update the patch so that MANGLE remains in use
>> but it will just change to a named asm constraint on Intel+Windows. Combine
>> that with a macro such as XMM_CLOBBERS_ONLY in order to add the necessary
>> asm interfaces and that should work fine.
>> 
>> That one thing I did notice though is that if the use of "m" constraints
>> with PIC is an issue then there are several locations in existing code that
>> dont seem to follow that. For instance in libavcodec/x86/cavsdsp.c the
>> macro QPEL_CAVSV1 uses a asm interface for MUL1 (which is variable ff_pw_96
>> and is assigned to %5). However MUL2 (which is variable ff_pw_42) is used
>> as a direct symbol through MANGLE. Since both these variables are defined
>> in the same place and are of the same type (uint64_t) then it appears there
>> are some inconsistencies in existing FFmpeg code. Ill leave all existing
>> instances of MANGLE but i thought id just point this out.
>> 
> 
> OK so I have redone the patch using named asm constraints and a macro to
> add them to the asm-interface (similar to XMM_CLOBBERS_ONLY as discussed).

I would feel more comfortable if this and only this part was a separate patch.
It is fairly simple to check whereas some of the other changes need further discussion.
I'd also suggest a different name for the define.
The name suggests that it would be added always.
I would suggest something like "MANGLE_REFERENCES" or at least something that makes it clearer that most of the time it expands to nothing at all.