[FFmpeg-devel] [libav-devel] [PATCH] x86inc: Avoid using eax/rax for storing the stack pointer

[Ronald S. Bultje]

Hi,

On Mon, Dec 26, 2016 at 4:53 AM, Henrik Gramner <henrik at gramner.com> wrote:

> On Mon, Dec 26, 2016 at 2:32 AM, Ronald S. Bultje <rsbultje at gmail.com>
> wrote:
> > I know I'm terribly nitpicking here for the limited scope of the comment,
> > but this only matters for functions that have a return value. Do you
> think
> > it makes sense to allow functions to opt out of this requirement if they
> > explicitly state to not have a return value?
>
> An opt-out would only be relevant on 64-bit Windows when the following
> criteria are true for a function:
>
> * Reserves exactly 6 registers
> * Reserves stack space with the original stack pointer stored in a
> register (as opposed to the stack)
> * Requires >16 byte stack alignment (e.g. spilling ymm registers to the
> stack)
> * Does not have a return value
>
> If and only if all of those are true this would result in one register
> being unnecessarily saved (the cost of which would likely be hidden by
> OoE). On other systems than WIN64 or if any of the conditions above is
> false an opt-out doesn't make any sense.
>
> Considering how rare that corner case is in combination with how
> fairly insignificant the downside is I'm not sure it makes that much
> sense to complicate the x86inc API further with an opt-out just for
> that specific scenario.
>

 Hm, OK, I think it affects unix64/x86-32 also when using 32-byte
alignment. We do use the stack pointer then. But let's ignore that for a
second, I think it's besides the point.

I think my hesitation comes from how I view x86inc.asm. There's two ways to
see it:
- it's a universal tool, like a compiler, to assist writing assembly
(combined with yasm/nasm as actual assembler);
or
- it's a local tool for ffmpeg/libav/x26[5], like libavutil/attributes.h,
to assist writing assembly.

If x86inc.asm were like a compiler, every micro-optimization, no matter the
benefit, would be important. If it were a local tool, we indeed wouldn't
care because ffmpeg spends most runtime for important use cases in other
areas. (There's obviously a grayscale in this black/white range that I'm
drawing out.) So having said that, patch is OK. If someone would later come
in to add something to take return value type (void vs. non-void) into
account, I would still find that helpful. :)

Ronald