| Message ID | 1583394508-21848-1-git-send-email-linjie.fu@intel.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | None | expand |
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Linjie Fu > Sent: Thursday, March 5, 2020 15:48 > To: ffmpeg-devel@ffmpeg.org > Cc: Xu, Guangxin <guangxin.xu@intel.com>; Fu, Linjie <linjie.fu@intel.com> > Subject: [FFmpeg-devel] [PATCH, v2 4/4] tests/checkasm: add overflow test > for hevc_add_res > > Add overflow test for hevc_add_res when int16_t coeff = -32768, > and doubled the test cases. > > The result of C is good, while ASM is not. > > To verify: > make fate-checkasm-hevc_add_res > ffmpeg/tests/checkasm/checkasm --test=hevc_add_res > > ./checkasm --test=hevc_add_res > checkasm: using random seed 679391863 > MMXEXT: > hevc_add_res_4x4_8_mmxext (hevc_add_res.c:69) > - hevc_add_res.add_residual [FAILED] > SSE2: > hevc_add_res_8x8_8_sse2 (hevc_add_res.c:69) > hevc_add_res_16x16_8_sse2 (hevc_add_res.c:69) > hevc_add_res_32x32_8_sse2 (hevc_add_res.c:69) > - hevc_add_res.add_residual [FAILED] > AVX: > hevc_add_res_8x8_8_avx (hevc_add_res.c:69) > hevc_add_res_16x16_8_avx (hevc_add_res.c:69) > hevc_add_res_32x32_8_avx (hevc_add_res.c:69) > - hevc_add_res.add_residual [FAILED] > AVX2: > hevc_add_res_32x32_8_avx2 (hevc_add_res.c:69) > - hevc_add_res.add_residual [FAILED] > checkasm: 8 of 14 tests have failed > > Signed-off-by: Xu Guangxin <guangxin.xu@intel.com> > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > [v2]: test 2x cases to make sure enough random residuals > are tested. > > tests/checkasm/hevc_add_res.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tests/checkasm/hevc_add_res.c > b/tests/checkasm/hevc_add_res.c > index e92c6b4..8c82ac1 100644 > --- a/tests/checkasm/hevc_add_res.c > +++ b/tests/checkasm/hevc_add_res.c > @@ -58,6 +58,7 @@ static void check_add_res(HEVCDSPContext h, int > bit_depth) > > randomize_buffers(res0, size); > randomize_buffers2(dst0, size); > + res0[0] = 0x8000;// overflow test > memcpy(res1, res0, sizeof(*res0) * size); > memcpy(dst1, dst0, sizeof(int16_t) * size); > > @@ -80,6 +81,7 @@ void checkasm_check_hevc_add_res(void) > > ff_hevc_dsp_init(&h, bit_depth); > check_add_res(h, bit_depth); > + check_add_res(h, bit_depth); > } > report("add_residual"); > } > -- Ping.
On Thu, Mar 05, 2020 at 03:48:28PM +0800, Linjie Fu wrote: > Add overflow test for hevc_add_res when int16_t coeff = -32768, > and doubled the test cases. > > The result of C is good, while ASM is not. > > To verify: > make fate-checkasm-hevc_add_res > ffmpeg/tests/checkasm/checkasm --test=hevc_add_res > > ./checkasm --test=hevc_add_res > checkasm: using random seed 679391863 > MMXEXT: > hevc_add_res_4x4_8_mmxext (hevc_add_res.c:69) > - hevc_add_res.add_residual [FAILED] > SSE2: > hevc_add_res_8x8_8_sse2 (hevc_add_res.c:69) > hevc_add_res_16x16_8_sse2 (hevc_add_res.c:69) > hevc_add_res_32x32_8_sse2 (hevc_add_res.c:69) > - hevc_add_res.add_residual [FAILED] > AVX: > hevc_add_res_8x8_8_avx (hevc_add_res.c:69) > hevc_add_res_16x16_8_avx (hevc_add_res.c:69) > hevc_add_res_32x32_8_avx (hevc_add_res.c:69) > - hevc_add_res.add_residual [FAILED] > AVX2: > hevc_add_res_32x32_8_avx2 (hevc_add_res.c:69) > - hevc_add_res.add_residual [FAILED] > checkasm: 8 of 14 tests have failed > > Signed-off-by: Xu Guangxin <guangxin.xu@intel.com> > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > [v2]: test 2x cases to make sure enough random residuals > are tested. > > tests/checkasm/hevc_add_res.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tests/checkasm/hevc_add_res.c b/tests/checkasm/hevc_add_res.c > index e92c6b4..8c82ac1 100644 > --- a/tests/checkasm/hevc_add_res.c > +++ b/tests/checkasm/hevc_add_res.c > @@ -58,6 +58,7 @@ static void check_add_res(HEVCDSPContext h, int bit_depth) > > randomize_buffers(res0, size); > randomize_buffers2(dst0, size); > + res0[0] = 0x8000;// overflow test > memcpy(res1, res0, sizeof(*res0) * size); > memcpy(dst1, dst0, sizeof(int16_t) * size); > > @@ -80,6 +81,7 @@ void checkasm_check_hevc_add_res(void) > > ff_hevc_dsp_init(&h, bit_depth); > check_add_res(h, bit_depth); > + check_add_res(h, bit_depth); Maybe iam mis-reading this diff but doesnt this run the same test twice instead of running it once with the frist elemet locked to 0x8000 and once freely floating ? thx [...]
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Michael Niedermayer > Sent: Monday, March 9, 2020 06:16 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v2 4/4] tests/checkasm: add overflow > test for hevc_add_res > > On Thu, Mar 05, 2020 at 03:48:28PM +0800, Linjie Fu wrote: > > Add overflow test for hevc_add_res when int16_t coeff = -32768, > > and doubled the test cases. > > > > The result of C is good, while ASM is not. > > > > To verify: > > make fate-checkasm-hevc_add_res > > ffmpeg/tests/checkasm/checkasm --test=hevc_add_res > > > > ./checkasm --test=hevc_add_res > > checkasm: using random seed 679391863 > > MMXEXT: > > hevc_add_res_4x4_8_mmxext (hevc_add_res.c:69) > > - hevc_add_res.add_residual [FAILED] > > SSE2: > > hevc_add_res_8x8_8_sse2 (hevc_add_res.c:69) > > hevc_add_res_16x16_8_sse2 (hevc_add_res.c:69) > > hevc_add_res_32x32_8_sse2 (hevc_add_res.c:69) > > - hevc_add_res.add_residual [FAILED] > > AVX: > > hevc_add_res_8x8_8_avx (hevc_add_res.c:69) > > hevc_add_res_16x16_8_avx (hevc_add_res.c:69) > > hevc_add_res_32x32_8_avx (hevc_add_res.c:69) > > - hevc_add_res.add_residual [FAILED] > > AVX2: > > hevc_add_res_32x32_8_avx2 (hevc_add_res.c:69) > > - hevc_add_res.add_residual [FAILED] > > checkasm: 8 of 14 tests have failed > > > > Signed-off-by: Xu Guangxin <guangxin.xu@intel.com> > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > --- > > [v2]: test 2x cases to make sure enough random residuals > > are tested. > > > > tests/checkasm/hevc_add_res.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/tests/checkasm/hevc_add_res.c > b/tests/checkasm/hevc_add_res.c > > index e92c6b4..8c82ac1 100644 > > --- a/tests/checkasm/hevc_add_res.c > > +++ b/tests/checkasm/hevc_add_res.c > > @@ -58,6 +58,7 @@ static void check_add_res(HEVCDSPContext h, int > bit_depth) > > > > randomize_buffers(res0, size); > > randomize_buffers2(dst0, size); > > + res0[0] = 0x8000;// overflow test > > memcpy(res1, res0, sizeof(*res0) * size); > > memcpy(dst1, dst0, sizeof(int16_t) * size); > > > > @@ -80,6 +81,7 @@ void checkasm_check_hevc_add_res(void) > > > > ff_hevc_dsp_init(&h, bit_depth); > > check_add_res(h, bit_depth); > > + check_add_res(h, bit_depth); > > Maybe iam mis-reading this diff but doesnt this run the same test > twice instead of running it once with the frist elemet locked to 0x8000 and > once freely floating ? > Running twice seems to be enough for the randomness even if it's the smallest block size (1/30 difference for 4x4 blocks). Current patch to simply run twice: 0x8000 + (size x size - 1) random residual; // 15 random residuals for 4x4 0x8000 + (size x size - 1) random residual; // 15 random residuals for 4x4 If modify to lock only one time: size x size random residual; // 16 random residuals for 4x4 0x8000 + (size x size - 1) random residual; // 15 random residuals for 4x4 However as you've pointed out, this introduced misunderstandings and may be treated as redundant code someday. It would be better to be treated more explicitly as you've suggested. Will update, thx. - Linjie
diff --git a/tests/checkasm/hevc_add_res.c b/tests/checkasm/hevc_add_res.c index e92c6b4..8c82ac1 100644 --- a/tests/checkasm/hevc_add_res.c +++ b/tests/checkasm/hevc_add_res.c @@ -58,6 +58,7 @@ static void check_add_res(HEVCDSPContext h, int bit_depth) randomize_buffers(res0, size); randomize_buffers2(dst0, size); + res0[0] = 0x8000;// overflow test memcpy(res1, res0, sizeof(*res0) * size); memcpy(dst1, dst0, sizeof(int16_t) * size); @@ -80,6 +81,7 @@ void checkasm_check_hevc_add_res(void) ff_hevc_dsp_init(&h, bit_depth); check_add_res(h, bit_depth); + check_add_res(h, bit_depth); } report("add_residual"); }
Add overflow test for hevc_add_res when int16_t coeff = -32768, and doubled the test cases. The result of C is good, while ASM is not. To verify: make fate-checkasm-hevc_add_res ffmpeg/tests/checkasm/checkasm --test=hevc_add_res ./checkasm --test=hevc_add_res checkasm: using random seed 679391863 MMXEXT: hevc_add_res_4x4_8_mmxext (hevc_add_res.c:69) - hevc_add_res.add_residual [FAILED] SSE2: hevc_add_res_8x8_8_sse2 (hevc_add_res.c:69) hevc_add_res_16x16_8_sse2 (hevc_add_res.c:69) hevc_add_res_32x32_8_sse2 (hevc_add_res.c:69) - hevc_add_res.add_residual [FAILED] AVX: hevc_add_res_8x8_8_avx (hevc_add_res.c:69) hevc_add_res_16x16_8_avx (hevc_add_res.c:69) hevc_add_res_32x32_8_avx (hevc_add_res.c:69) - hevc_add_res.add_residual [FAILED] AVX2: hevc_add_res_32x32_8_avx2 (hevc_add_res.c:69) - hevc_add_res.add_residual [FAILED] checkasm: 8 of 14 tests have failed Signed-off-by: Xu Guangxin <guangxin.xu@intel.com> Signed-off-by: Linjie Fu <linjie.fu@intel.com> --- [v2]: test 2x cases to make sure enough random residuals are tested. tests/checkasm/hevc_add_res.c | 2 ++ 1 file changed, 2 insertions(+)