| Message ID | AS8P250MB074426E9702BF42E69CE5B5D8F4F2@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | Accepted |
| Commit | 7ed1e806e76092df27d7f984a554546a1cf1a175 |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/x86/simple_idct: Empty MMX state in ff_simple_idct_mmx | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | fail | Make fate failed |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | fail | Make fate failed |
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
> Will apply this patch tomorrow unless there are objections. > LGTM
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
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(+)