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

[Matt Oliver]

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 part --------------
A non-text attachment was scrubbed...
Name: 2-4-Fix-libmpcodecs-data-align-for-cross-platform-co.patch
Type: application/octet-stream
Size: 3895 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140206/4813cf80/attachment.obj>