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

[Michael Niedermayer]

On Mon, Mar 17, 2014 at 03:32:59PM +1100, Matt Oliver wrote:
> >
> > If this patchset is applied it will need someone to maintain it
> > otherwise it will probably be more broken than working most of the
> > time.
> 
> 
> Figured as much. The reason ive been updating these patches for the last
> few months is that I have my own repo that I use for compiling ffmpeg for
> Windows that I use in a project im working on. So for a while atleast I
> will be compiling ffmpeg using icl (and the proposed changes) so its easier
> for me to locate bugs and I will be fixing them in my own repo so its not
> to much effort to provide them upstream.
> The rebasing is kind of a pain, but thats mainly because these patches were
> initially written up last year and have been working in my repo for ages.
> The rebasing is just so I can get the patches relative to upstream head so
> that you guys can apply it easily. And all the rebasing is pretty much
> always because configure is modified pretty much every day. Once the
> configure changes are in from the initial patches future updates will
> mostly be just re-tweaking some asm and shouldn't have so many issues and
> wouldnt need constant rebasing.

> Either way consider me able and willing to
> support the icl asm for awhile.

ok, thanks


> 
> > +inline_asm_nonlocal_labels_deps="inline_asm"
> > > +
> > >  # system capabilities
> > >  log2_deps="!libc_msvcrt"
> > >
> > is this actually needed ?
> 
> 
> Possibly not, the nonlocal labels should not be used if inline asm is
> disabled as the code will never get called. But as far as the config.h
> header is concerned I figured for completeness there is no point having the
> option enabled in the header when it clearly cant be because all inline asm
> is disabled. I assumed it was logical to include it but if you want it
> removed anyway let me know.

i think its better not to have unneeded code in there, so if its
needed it should stay, if not it should be removed


> 
> this looks a bit more complex than needed
> > you should be able to just unconditionally add the
> > NAMED_CONSTRAINTS_ADD() to the existing macro
> 
> 
> I though of doing that but there are several locations ( line 392, 759, 781
> to name a few) where there are no additional named constraints added. There
> are also several locations where only 1 named constraint is required. So by
> adding all of them to the existing macro then it would be adding named
> constraints that are only needed in some but not all instances. This raises
> compilation warnings about unused named references so I opted to keep
> things as clean and efficient as possible. So again Ill leave this up to
> you. If youd prefer the possible safety of just adding all named
> constraints to the existing macro but have some of them be unused at
> various times then let me know.
> This also goes for the RGB_PACK24_B_OPERANDS as combining them will result
> in additional unneeded constraints being added to asm blocks that dont use
> them.

These constraints are there because of limitions/shortcommings of
intels asm() support on windows.
IMO we should keep such workarounds to the minimum possible


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140317/a041481c/attachment.asc>