On Mon, 28 Mar 2022, Ben Avison wrote:
On 25/03/2022 22:53, Martin Storsjö wrote:
On Fri, 25 Mar 2022, Ben Avison wrote:
+#define
CHECK_LOOP_FILTER(func) \
+ do
{ \
+ if (check_func(h.func, "vc1dsp." #func))
{ \
+ declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *, int,
int); \
+ for (int count = 1000; count > 0; --count)
{ \
+ int pq = rnd() % 31 +
1; \
+ RANDOMIZE_BUFFER8_MID_WEIGHTED(filter_buf, 24 *
24); \
+ call_ref(filter_buf0 + 4 * 24 + 4, 24,
pq); \
+ call_new(filter_buf1 + 4 * 24 + 4, 24,
pq); \
+ if (memcmp(filter_buf0, filter_buf1, 24 *
24)) \
+
fail(); \
+
} \
+
} \
+ for (int j = 0; j < 24;
++j) \
+ for (int i = 0; i < 24;
++i) \
+ filter_buf1[24*j + i] = 0x60 + 0x40 * (i >= 4 && j >=
4); \
+ if (check_func(h.func, "vc1dsp." #func "_bestcase"))
{ \
+ declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *, int,
int); \
+ bench_new(filter_buf1 + 4 * 24 + 4, 24,
1); \
+ (void)
checked_call; \
+
} \
+ if (check_func(h.func, "vc1dsp." #func "_worstcase"))
{ \
+ declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *, int,
int); \
+ bench_new(filter_buf1 + 4 * 24 + 4, 24,
31); \
+ (void)
checked_call; \
+
} \
(not a full review, just something that cropped up in initial build
testing)
Why do you have the "(void) checked_call;" here? The checked_call isn't
something that is universally defined; its availability depends on the
OS/arch combinations, on other combinations, call_new/call_ref just call
the function straight away without a wrapper.
OK, I missed that subtlety. My aim was to avoid the "unused variable"
compiler warnings generated as a result of there being twice as many
benchmark tests as correctness tests.
Oh, I see. I just ran into it when trying to compile on macOS, then edited
it out and saw that it built fine there, but didn't try building for other
platforms with the same modification.
I believe we need separate calls of check_func() to initialise the cycle
counts for each benchmark, and copying the sequence of macros from
checkasm/blockdsp.c,
FWIW I think blockdsp.c might have been a bad example in that regard, as
it expands the whole testcase with macros. (I chose it mainly as it was
one of the shortest testcases.)
I think e.g. vp8dsp would have been a better example - with the toplevel
checkasm_check_*() function just calling individual functions for the
tests for various function groups. As check_func() can take a format
string, you don't usually need the macro expansion for filling that in.
I was placing the declare_func_emms() invocations inside the if block
that used check_func(). That meant that checked_call was initialised,
but since the correctness test (call_ref / call_new) was in a different
block scope, this checked_call declaration was never used.
Upon further investigation, I think it's valid to move the
declare_func_emms() invocation up to the next largest block scope. That
means it would only appear once rather than 3 times, and it wouldn't
need the cast-to-void any more. Please do correct me if I'm wrong.
Yes, that seems correct to do. And looking at other examples, e.g. vp8dsp,
that also uses such a structure, with declare_func_*() outside of
check_func() - in a function like check_loopfilter_simple().
// Martin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
