[FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop

[wm4]

On Mon, 24 Oct 2016 21:19:46 +0200
Andreas Cadhalpun <andreas.cadhalpun at googlemail.com> wrote:

> On 24.10.2016 16:14, Ronald S. Bultje wrote:
> > On Mon, Oct 24, 2016 at 8:47 AM, wm4 <nfxjfg at googlemail.com> wrote:  
> >> On Mon, 24 Oct 2016 07:54:47 -0400
> >> "Ronald S. Bultje" <rsbultje at gmail.com> wrote:  
> >>> On Mon, Oct 24, 2016 at 3:36 AM, wm4 <nfxjfg at googlemail.com> wrote:  
> >>>> I was under the impression that it is UB to have the FPU in MMX state
> >>>> at any time while in C, not just while e.g. calling the stdlib. Maybe I
> >>>> got that wrong (how would MMX intrinsics even work?) - can anyone shed
> >>>> light on the exact requirements? (Possibly again, sorry.)  
> >>>
> >>> I'm under the impression that it's part of the calling convention. That  
> >> is,  
> >>> any code anywhere (including mmx intrinsics, indeed) can - when called -
> >>> expect the state to be cleared by the caller, just like you'd expect
> >>> eax/edx to be caller-save (whereas esi/edi are callee-save).
> >>>
> >>> However, if you never call external code (including intrinsics), you can
> >>> ignore this, just as you can ignore / create your own calling
> >>> convention (remember fastcall etc.?). However, when calling any external
> >>> code, this could (in theory) crash; it's just that right now it only
> >>> crashes with musl when calling malloc/free. So basically, ffmpeg has its
> >>> own calling convention, and manually calling emms_c() fixes "ffmpeg"
> >>> calling convention to be compatible with "standard" calling  
> >> convention...?
> >>
> >> It can't really be a calling convention unless the compiler is aware of
> >> it?  
> 
> It is defined as part of the System V Application Binary Interface [1]:
> "The CPU shall be in x87 mode upon entry to a function. Therefore, every
> function that uses the MMX registers is required to issue an emms or femms
> instruction after using MMX registers, before returning or calling another
> function."

I mean FFmpeg can't make up its own calling convention without the
compiler's knowledge.

But thanks for reminding me about this but of the sysv ABI. The
paragraph you quoted is actually very clear about the requirements. It
means FFmpeg can barely do anything and remain standard compliant: a
ASM function must, according to the calling convention, reset the MMX
state when returning.

What FFmpeg does here was misdesigned from the very start.

> >> We're doing things behind the compiler's back here. The safest
> >> assumption would be that leaving the FPU state invalid while in C is
> >> not allowed, period.  
> 
> That's what the standard says.
> 
> >> The next safest assumption is that it's fine as long as we explicitly
> >> don't use floating point or call external functions that aren't
> >> MMX-aware. This would mean calling any libc functions or user callbacks
> >> (including indirectly through e.g. av_log) is not fine.  
> 
> This is probably OK in practice and likely has a significant performance
> benefit.

Seems like it.

The compiler still could generate code using the FPU state at any
point, though, unless maybe there is an inline asm block. No function
can put the FPU into MMX mode, because any MMX using function must have
called emms before returning. Consequently, only compiler-known
intrinsics or opaque inline asm block could clobber the FPU state. The
latter because the compiler doesn't inspect asm blocks.

> 
> Best regards,
> Andreas
> 
> 
> 1: https://software.intel.com/sites/default/files/article/402129/mpx-linux64-abi.pdf