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

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

[Andreas Rheinhardt]

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 the avdct API. Without this, the following fragment will
assert (on x86_32, as ff_simple_idct_mmx is not used on x64):
    int16_t __attribute__ ((aligned (16))) block[64];
    AVDCT *dct = avcodec_dct_alloc();
    dct->idct_algo = FF_IDCT_AUTO;
    avcodec_dct_init(dct);
    dct->idct(block);
    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 982b2f0bbb..4139b6dab5 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
 
 INIT_XMM sse2
-- 
2.34.1

_______________________________________________
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".

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

[Andreas Rheinhardt]

Andreas Rheinhardt:
> 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 the avdct API. Without this, the following fragment will
> assert (on x86_32, as ff_simple_idct_mmx is not used on x64):
>     int16_t __attribute__ ((aligned (16))) block[64];
>     AVDCT *dct = avcodec_dct_alloc();
>     dct->idct_algo = FF_IDCT_AUTO;
>     avcodec_dct_init(dct);
>     dct->idct(block);
>     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 982b2f0bbb..4139b6dab5 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
>  
>  INIT_XMM sse2

Will apply this patch tomorrow unless there are objections.

- Andreas

_______________________________________________
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".

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

[Kieran Kunhya]

> Will apply this patch tomorrow unless there are objections.
>

LGTM
_______________________________________________
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".