Re: [FFmpeg-devel] [PATCH 1/9] avcodec/x86/simple_idct: Empty MMX state in ff_simple_idct_mmx

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 2183 bytes --]

On Thu, Oct 06, 2022 at 02:54:30AM +0200, Andreas Rheinhardt wrote:
> We currently mostly do not empty the MMX state in our MMX
> DSP functions; instead we only do so before code that might
> be using x87 code. This is a violation of the System V i386 ABI
> (and maybe of other ABIs, too):
> "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." (See 2.2.1 in [1])
> This patch does not intend to change all these functions to abide
> by the ABI; it only does so for ff_simple_idct_mmx(), as this
> function can by called by external users, because it is exported
> via AVDCT (i.e. via avcodec_dct_init()). Without this, the following
> fragment will assert (in i386):
>     av_force_cpu_flags(AV_CPU_FLAG_MMX | AV_CPU_FLAG_MMXEXT);
>     int16_t *blk = av_malloc(64 * sizeof(*blk));
>     AVDCT *avdct = avcodec_dct_alloc();
>     avcodec_dct_init(avdct);
>     avdct->idct(blk);
>     av_assert0_fpu();
> 
> [1]: https://raw.githubusercontent.com/wiki/hjl-tools/x86-psABI/intel386-psABI-1.1.pdf
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/x86/simple_idct.asm | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/x86/simple_idct.asm b/libavcodec/x86/simple_idct.asm
> index dcf0da6df1..e3a29efc33 100644
> --- a/libavcodec/x86/simple_idct.asm
> +++ b/libavcodec/x86/simple_idct.asm
> @@ -845,6 +845,7 @@ INIT_MMX mmx
>  
>  cglobal simple_idct, 1, 2, 8, 128, block, t0
>      IDCT
> +    emms
>  RET

as comments where requested

What speed impact does this have on old CPUs ?
can this be made compile time optional so someone building for an
old machiene can avoid the speedhit while ignoring the ABI (which
worked fine)
(this case would then require a 2nd function with the emms for AVDCT)

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".