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

[Matt Oliver]

On 6 February 2014 12:35, Matt Oliver <protogonoi at gmail.com> wrote:

> On 6 February 2014 05:45, Reimar Döffinger <Reimar.Doeffinger at gmx.de>wrote:
>
>> On Wed, Feb 05, 2014 at 11:41:38AM +1100, Matt Oliver wrote:
>> > Cheers for the feedback. Ill split up the patches to make things
>> easier. To
>> > start off I have split the libmpcodec patches into some smaller easily
>> > digestible chunks.
>> >
>> > Patch 1: Changes a couple of inline asm labels for using named labels to
>> > standard numbered labels. This has no affect but to allow for
>> compilation
>> > with compilers that dont support named labels.
>>
>> If you use numbered labels I believe you need to append the jump
>> direction.
>> I.e.
>> jnz 1b
>> instead of
>> jnz 1
>> if you mean the 1: label that comes before.
>> At least that is what I see in e.g. libavcodec/x86/snowdsp.c and many
>> more files.
>>
>> > > Well, having the DECLARE_ALIGNED changes separately would be nice
>> since I
>> > think that part can be applied without further review.
>> >
>> > Patch 2: Is just the declare aligned fixes. This uses the existing cross
>> > platform specific declare aligned macro instead of the gcc only
>> attribute.
>> > Seperated as Reimar suggested.
>>
>> You seem to have lost the very first "static". Otherwise it looks ok to
>> me.
>>
>> > Patch 3: Removes unnecessary commas from some inline asm. Leaving the
>> > commas in with nothing following them causes icl compilation errors.
>> > removing them has no impact on anything else as ive seen this used else
>> > where in ffmpeg so this should have no impact.
>>
>> Looks good to me as well.
>> If nobody has any other comments I will try to get it applied to MPlayer
>> soon to try to keep the codebases in sync (or if I'm too slow
>> anyone with commit access there is of course free to apply them).
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
> > If you use numbered labels I believe you need to append the jump
> > direction.
>
> You only have to do this if you have multiple labels with the same name.
> In this case the forward/backward identifiers are so the assembler knows
> which label you are referring to (closest in front or closest behind).
> Since the labels in the patch are all unique then there is only one so we
> dont need to specify which direction to find it.
>
> > You seem to have lost the very first "static". Otherwise it looks ok to
> > me.
>
> That I did, fixed in updated patch attached. I changed to using the
> predefined ASM_CONSTANT macro that auto includes static const.
>
>
Next round of patches, this time for everything else (i.e. non
libmpcodecs). I broke the whole thing up into some smaller easier to handle
patches.

Patch 1: Adds some missing external declarations. Nice and easy this one.

Patch 2: Adds a configuration check and option
for HAVE_INLINE_ASM_NONLOCAL_LABELS which checks for the ability to use
named nonlocal labels in inline asm.

Patch 3: This is the main one and it adds support for using named
references in place of direct symbol references. This includes the changes
to MANGLE.

Patch 4: This adds a work around for get_cabac not working on Intel. This
was previously using the BROKEN_COMPILER define but I renamed it to the
more descriptive BROKEN_REGISTER_ALLOCATION (because the error is due to
register allocation failing so I figured that was clear enough now).

Patch 5: 2 very minor fixes for icl. First changes the order
of yuv2rgb_template inclusion (as previously discussed this is necessary).
And the other explicitly uses eax in movzbl instead of REG_a as movzbl only
works on eax and not rax anyway so this avoids a compiler error.

Patch 6: Adds a configure check for availability of cdq over cltd and
creates a define accordingly.

In addition to all those there is the final libmpcodec patch which adds the
named references added in Patch 3 above.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1-6-Add-missing-external-declarations.patch
Type: application/octet-stream
Size: 2408 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140208/be001c17/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 2-6-Check-for-nonlocal-inline-asm-labels.patch
Type: application/octet-stream
Size: 1884 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140208/be001c17/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 3-6-Check-for-inline-asm-direct-symbol-reference-sup.patch
Type: application/octet-stream
Size: 33813 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140208/be001c17/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 4-6-Fix-for-broken-register-allocation-issues-with-I.patch
Type: application/octet-stream
Size: 3028 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140208/be001c17/attachment-0003.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 5-6-Minor-fixes-for-Intel-on-windows-compilation.patch
Type: application/octet-stream
Size: 2762 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140208/be001c17/attachment-0004.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 6-6-Fix-for-missing-intel-nmemonic-support-cdq.patch
Type: application/octet-stream
Size: 3526 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140208/be001c17/attachment-0005.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 4-4-Fix-libmpcodecs-inline-asm-on-ICL.patch
Type: application/octet-stream
Size: 3390 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140208/be001c17/attachment-0006.obj>