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

[Reimar Döffinger]

On Mon, Feb 03, 2014 at 09:16:47PM +1100, Matt Oliver wrote:
> On 3 February 2014 18:45, Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
> > On 03.02.2014, at 08:13, Matt Oliver <protogonoi at gmail.com> wrote:
> > > Patch 3: This is the inline asm changes specific for libmpcodecs. If you
> > > want me to submit this separately then let me know.
> >
> > Well, having the DECLARE_ALIGNED changes separately would be nice since I
> > think that part can be applied without further review.
> >
> 
> I can split the libmpcodecs patch into 2 if needed.

"Needed" is an exaggeration, but the larger the patch, the fewer
people will have time to review, the more review comments you'll
get all over the place, it makes debugging the changes later harder etc.
Splitting is a simple way to reduce the amount of code changes
you have locally.

> Although I must have missed the issue with cdq being raised (or forgot).

Mentioned it in both of my earlier partial reviews.

> Given cltd will not work at all in Intel, both work for gcc and apparently
> cdq does not work with other compilers what is the recommend way to get
> around that? A define would be the obvious one but a little ugly. Any
> suggestions?

Define doesn't sound so bad, but to be honest this change is in the
middle of a huge number of other changes so I have 0 overview and
feel unable to really get an idea of what would be appropriate.
Which is why I suggest splitting patches: reduce the complexity
of the problem by solving it step by step instead of all in one go.
With the trivial stuff first since that is unlikely to need changes
and should go in quickly.

> Also if BROKEN_COMPILER is bad naming what would you suggest? Unfortunately
> its one of those situations were the compiler is behaving badly so I
> figured the name was appropriate.

Well, the "problem" is that a compiler can be broken in hundreds of
ways, something more specific would seem helpful.
Since I don't know/haven't checked what exactly it is about I don't
have any suggestion, but in general the question to ask is:
If you look at the code in 5 years, what name might help you understand
what goes on?
I'm fairly certain that "BROKEN_COMPILER" will be almost as helpful a
name for understanding as just calling it "FJSNVGFRVB".
But yes, it's not a big deal, just a suggestion to think about.