diff mbox series

[FFmpeg-devel] checkasm: store and associate contexts with functions and use it for av_tx

Message ID MrJ3Ebm--3-2@lynne.ee
State New
Headers show
Series [FFmpeg-devel] checkasm: store and associate contexts with functions and use it for av_tx | expand

Checks

Context Check Description
andriy/commit_msg_x86 warning Please wrap lines in the body of the commit message between 60 and 72 characters.
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/commit_msg_ppc warning Please wrap lines in the body of the commit message between 60 and 72 characters.
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Lynne Dec. 19, 2021, 7 p.m. UTC 
checkasm: store and associate contexts with functions and use it for av_tx

The issue is the following:
- checkasm/av_tx.c initializes a context and a function
- check_func() receives the function only and returns NULL
- checkasm/av_tx.c runs again with a different CPU flag and creates a new function and context
- check_func() sees a new function for testing and sets func_ref to the function it first received
- call_ref() and call_new() get called with the same, new context

This means that av_tx contexts had to be compatible with both C and assembly functions.
And so each context difference had to be generated twice and duplicated to keep checkasm
working.

The commit introduces a new *cont in the checkasm function struct, which is associated
with each function. Code that doesn't need an additional context requires no changes,
as we use wrapper macros.

A downside is that there's no way to free contexts until the very end. However, as
checkasm is a testing infrastructure, I think it's reasonable, and we're talking a few
tens of kilobytes of heap memory as we only test a limited number of smaller transforms.

Patch attached.
Subject: [PATCH] checkasm: store and associate contexts with functions and use
 it for av_tx

The issue is the following:
 - checkasm/av_tx.c initializes a context and a function
 - check_func() receives the function only and returns NULL
 - checkasm/av_tx.c runs again with a different CPU flag and creates a new function and context
 - check_func() sees a new function for testing and sets func_ref to the function it first received
 - call_ref() and call_new() get called with the same, new context

This means that av_tx contexts had to be compatible with both C and assembly functions.
And so each context difference had to be generated twice and duplicated to keep checkasm
working.

The commit introduces a new *cont in the checkasm function struct, which is associated
with each function. Code that doesn't need an additional context requires no changes,
as we use wrapper macros.

A downside is that there's no way to free contexts until the very end. However, as
checkasm is a testing infrastructure, I think it's reasonable, and we're talking a few
tens of kilobytes of heap memory as we only test a limited number of smaller transforms.
---
 tests/checkasm/av_tx.c    | 10 +++++-----
 tests/checkasm/checkasm.c | 21 ++++++++++++---------
 tests/checkasm/checkasm.h | 10 +++++++---
 3 files changed, 24 insertions(+), 17 deletions(-)
diff mbox series

Patch

diff --git a/tests/checkasm/av_tx.c b/tests/checkasm/av_tx.c
index 178fb61972..afa5dbac72 100644
--- a/tests/checkasm/av_tx.c
+++ b/tests/checkasm/av_tx.c
@@ -58,11 +58,11 @@  static const int check_lens[] = {
                 return;                                                           \
             }                                                                     \
                                                                                   \
-            if (check_func(fn, PREFIX "_%i", len)) {                              \
+            if (check_func_cont(fn, tx, PREFIX "_%i", len)) {                     \
                 num_checks++;                                                     \
                 last_check = len;                                                 \
-                call_ref(tx, out_ref, in, sizeof(DATA_TYPE));                     \
-                call_new(tx, out_new, in, sizeof(DATA_TYPE));                     \
+                call_ref(cont_ref, out_ref, in, sizeof(DATA_TYPE));               \
+                call_new(cont_new, out_new, in, sizeof(DATA_TYPE));               \
                 if (CHECK_EXPRESSION) {                                           \
                     fail();                                                       \
                     break;                                                        \
@@ -70,11 +70,11 @@  static const int check_lens[] = {
                 bench_new(tx, out_new, in, sizeof(DATA_TYPE));                    \
             }                                                                     \
                                                                                   \
-            av_tx_uninit(&tx);                                                    \
+            tx = NULL;                                                            \
             fn = NULL;                                                            \
         }                                                                         \
                                                                                   \
-        av_tx_uninit(&tx);                                                        \
+        tx = NULL;                                                                \
         fn = NULL;                                                                \
                                                                                   \
         if (num_checks == 1)                                                      \
diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
index 90d080de02..33b7226ff3 100644
--- a/tests/checkasm/checkasm.c
+++ b/tests/checkasm/checkasm.c
@@ -246,6 +246,7 @@  static const struct {
 typedef struct CheckasmFuncVersion {
     struct CheckasmFuncVersion *next;
     void *func;
+    void *cont;
     int ok;
     int cpu;
     CheckasmPerf perf;
@@ -735,12 +736,11 @@  int main(int argc, char *argv[])
 }
 
 /* Decide whether or not the specified function needs to be tested and
- * allocate/initialize data structures if needed. Returns a pointer to a
- * reference function if the function should be tested, otherwise NULL */
-void *checkasm_check_func(void *func, const char *name, ...)
+ * allocate/initialize data structures if needed. Sets *func_ref and *ctx_ref to the
+ * reference function and context if so, otherwise NULL */
+int checkasm_check_func(void *func, void **func_ref, void *cont, void **cont_ref, const char *name, ...)
 {
     char name_buf[256];
-    void *ref = func;
     CheckasmFuncVersion *v;
     int name_length;
     va_list arg;
@@ -750,7 +750,7 @@  void *checkasm_check_func(void *func, const char *name, ...)
     va_end(arg);
 
     if (!func || name_length <= 0 || name_length >= sizeof(name_buf))
-        return NULL;
+        return 0;
 
     state.current_func = get_func(&state.funcs, name_buf);
     state.funcs->color = 1;
@@ -761,10 +761,12 @@  void *checkasm_check_func(void *func, const char *name, ...)
         do {
             /* Only test functions that haven't already been tested */
             if (v->func == func)
-                return NULL;
+                return 0;
 
-            if (v->ok)
-                ref = v->func;
+            if (v->ok) {
+                *func_ref = v->func;
+                *cont_ref = v->cont;
+            }
 
             prev = v;
         } while ((v = v->next));
@@ -773,6 +775,7 @@  void *checkasm_check_func(void *func, const char *name, ...)
     }
 
     v->func = func;
+    v->cont = cont;
     v->ok = 1;
     v->cpu = state.cpu_flag;
     state.current_func_ver = v;
@@ -780,7 +783,7 @@  void *checkasm_check_func(void *func, const char *name, ...)
     if (state.cpu_flag)
         state.num_checked++;
 
-    return ref;
+    return !!(*func_ref);
 }
 
 /* Decide whether or not the current function needs to be benchmarked */
diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
index 68b0697d3e..e65deabd79 100644
--- a/tests/checkasm/checkasm.h
+++ b/tests/checkasm/checkasm.h
@@ -87,7 +87,7 @@  void checkasm_check_videodsp(void);
 
 struct CheckasmPerf;
 
-void *checkasm_check_func(void *func, const char *name, ...) av_printf_format(2, 3);
+int checkasm_check_func(void *func, void **func_ref, void *cont, void **cont_ref, const char *name, ...) av_printf_format(5, 6);
 int checkasm_bench_func(void);
 void checkasm_fail_func(const char *msg, ...) av_printf_format(1, 2);
 struct CheckasmPerf *checkasm_get_perf_context(void);
@@ -111,11 +111,15 @@  extern AVLFG checkasm_lfg;
 #define rnd() av_lfg_get(&checkasm_lfg)
 
 static av_unused void *func_ref, *func_new;
+static av_unused void *cont_ref, *cont_new;
 
 #define BENCH_RUNS 1000 /* Trade-off between accuracy and speed */
 
-/* Decide whether or not the specified function needs to be tested */
-#define check_func(func, ...) (func_ref = checkasm_check_func((func_new = func), __VA_ARGS__))
+/* Decide whether or not the specified function needs to be tested, returns !0 if so, otherwise 0 */
+#define check_func(func, ...) (checkasm_check_func((func_new = func), &func_ref, NULL, &cont_ref, __VA_ARGS__))
+
+/* Same as above, only takes a function-specific context */
+#define check_func_cont(func, cont, ...) (checkasm_check_func((func_new = func), &func_ref, (cont_new = cont), &cont_ref, __VA_ARGS__))
 
 /* Declare the function prototype. The first argument is the return value, the remaining
  * arguments are the function parameters. Naming parameters is optional. */