[FFmpeg-devel] [PATCH 1/4] x86inc: Support arbitrary stack alignments

[Ronald S. Bultje]

Hi,

On Mon, Aug 3, 2015 at 8:50 AM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:

> On Mon, Aug 3, 2015 at 10:31 AM, Henrik Gramner <henrik at gramner.com>
> wrote:
> > On Mon, Aug 3, 2015 at 2:18 AM, Ronald S. Bultje <rsbultje at gmail.com>
> wrote:
> >> So, I think the code changes themselves look mostly healthy. Is there a
> >> behavioural difference before/after this patch? (Like: were there bugs
> in
> >> the original code, or does this change behaviour of previous code in a
> >> significant way?)
> >
> > Should only be what's in the commit message; "Previously alignment
> > would occur either before or after allocating stack space depending on
> > whether manual alignment was required or not." which I guess you could
> > classify as a bug (it certainly wasn't a sensible behavior). It's the
> > reason for why the weird deblock stack allocation for example existed
> > in the first place.
> >
> > So anything relying on the previous alignment behavior of automatic
> > stack allocation using cglobal would be affected, other than that it
> > shouldn't make any difference since ffmpeg doesn't use >16-byte stack
> > alignment.
> >
> > I can only compile ffmpeg with --disable-programs when using
> > msys2/msvc2015 (ffmpeg.c(437): error C2039: '_cnt': is not a member of
> > '_iobuf'). Not sure if I'm doing something wrong, but if someone is
> > able to test that better that would be nice.
>
> msvc2015 support is unfortunately still broken. I'll send a patch
> later for this.
>
> Otherwise, I tested the patch with msvc 2013 32-bit, and fate passed fine.
> If there is something else I should specifically test which may not be
> covered by fate, let me know.


I think it tests things sufficiently well, so go ahead and apply. Fate will
pick up some remaining items (like testing with restricted cpuflags).

Tnx,
Ronald