From patchwork Sun Dec 19 19:00:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lynne X-Patchwork-Id: 32724 Delivered-To: ffmpegpatchwork2@gmail.com Received: by 2002:a6b:cd86:0:0:0:0:0 with SMTP id d128csp3680914iog; Sun, 19 Dec 2021 11:00:36 -0800 (PST) X-Google-Smtp-Source: ABdhPJzefuL0KHr+7VGJnkJ78vh0ZXJ10LR66PRWJFggpzH/CM4JfUUFmZhcUwzTMutX64rrOHGK X-Received: by 2002:a05:6402:2c4:: with SMTP id b4mr9185751edx.364.1639940436701; Sun, 19 Dec 2021 11:00:36 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1639940436; cv=none; d=google.com; s=arc-20160816; b=woIgmSIH7d4+AmXGnPfi2+nzBlWK54TPugPE26RWP1qQRPimpWi8V6Q3X0MkifhhtH MoO4NlZwwIz1wZ7yrxmcPQlMlHBPgacN0/UihgTPZrsiUgTS311V+Jxn34W1cCvP3wl8 0UzUDr+wm0Cyw91Xkk3JRIwelToSuSwLEeNkOjbZHGe+FRCyxt19Dw40+BSJvsdTGP3t aS/UEQz9/AVRoRvTuJa9w1ugg7cRlHXT3/avquJy7o/MMv1udWV0umXgBPI9DE91CB7d H5yaYIdb2qKsRbHUQtHxcWQYAxG7SWK8v4bk1zJ+IE4hrZzq9Go64pIs0+i7vvvw7zzf 1oaQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:message-id:to:from:date:dkim-signature:delivered-to; bh=uxfr4PrgVl+ncHuBgvCWaO7PdMRva3dLfLFu6OhLHmQ=; b=mjaQTUA+DXREMN46js+gSwptdPN6GjKBWlF1Epr/LPwaJ2FximoFE21TPdJyM1xvnm d0vFGWFT6kRujGEBEJtbIqSG4ocM6o0BZumqZKZ3WFQh745l3WMmZGk9Hgp7uXnJqK29 HGtv2eYZw9z0y9/wgTvOaz0OiDPGDezKoS5tl2mpTDXEnvT+2HpGuUuViiN5rpB0lTva iQNdnKeA4OBQr7mRu2MNTISel2RVhmCrcB2k5o91P6o0xOQfL1YLfHaD9iojNpD/QQNK HbPwteCewnetg1CbHy3jc6kDZQfKlsmz7a/YRagbDrMNDhCmm+PkzwHzDw1fvogrOHba 0mbw== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@lynne.ee header.s=s1 header.b=iXPcrv0i; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=lynne.ee Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id o21si9533479ejy.696.2021.12.19.11.00.35; Sun, 19 Dec 2021 11:00:36 -0800 (PST) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@lynne.ee header.s=s1 header.b=iXPcrv0i; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=lynne.ee Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C9EF968AEEB; Sun, 19 Dec 2021 21:00:31 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from w4.tutanota.de (w4.tutanota.de [81.3.6.165]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B7D1E68ADF8 for ; Sun, 19 Dec 2021 21:00:24 +0200 (EET) Received: from w3.tutanota.de (unknown [192.168.1.164]) by w4.tutanota.de (Postfix) with ESMTP id 4A7991060152 for ; Sun, 19 Dec 2021 19:00:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1639940423; s=s1; d=lynne.ee; h=From:From:To:To:Subject:Subject:Content-Description:Content-ID:Content-Type:Content-Type:Content-Transfer-Encoding:Cc:Date:Date:In-Reply-To:MIME-Version:MIME-Version:Message-ID:Message-ID:Reply-To:References:Sender; bh=MP4fqKTYi7RAi/bSW6NWAJ4QbginnRTFN1TvIK9/qnQ=; b=iXPcrv0isjR1rqILfk21TrRYcE+oIIKwmMQfxj1XVrQNepwNCI/MgPLBASWe4AGP 3Q710u9jZSmyY5MGamNDJ7E3K85Ot+LHvX2YhkXBEzk+Hubb6n4umJrSk9il45Ejvyb LAT/3kWqIt9VeaGD0oTfjHnsl9U+xhov0xS7hA8qaV0XOmsXWGE9IwjmOy+5OHyp1YP Gg1dGSiLw4qkRfvY8MH7JmqFyb8qKwyeBoYztXK5ka1IdYENCbYsCTmWKskTb+EVD6y 1EZ3guOGyuv1sCjE0550rXVVAptB51LU1VS6Nol5/FQ7p1hICEs1+DfPfes3XJVaUNg MfyLhU7/Ww== Date: Sun, 19 Dec 2021 20:00:23 +0100 (CET) From: Lynne To: Ffmpeg Devel Message-ID: MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] checkasm: store and associate contexts with functions and use it for av_tx X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" X-TUID: z/pmNmTIojkf 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 --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. */