On Mon, Aug 26, 2024, at 12:41, Ramiro Polla wrote:
> On Sun, Aug 25, 2024 at 7:52 PM J. Dekker <j...@itanimul.li> wrote:
>> On Thu, Aug 22, 2024, at 16:57, Ramiro Polla wrote:
>> > On Wed, Aug 21, 2024 at 1:26 PM J. Dekker <j...@itanimul.li> wrote:
>> >>
>> >> Port dav1d's checkasm output format to FFmpeg's checkasm, includes
>> >> relative speedups and aligns results.
>> >>
>> >> Signed-off-by: J. Dekker <j...@itanimul.li>
>> >> ---
>> >> tests/checkasm/checkasm.c | 53 +++++++++++++++++++++++++++++++++++----
>> >> 1 file changed, 48 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
>> >> index c5c1eeb07a..f43c1c50f0 100644
>> >> --- a/tests/checkasm/checkasm.c
>> >> +++ b/tests/checkasm/checkasm.c
>> >> @@ -18,6 +18,31 @@
>> >> * You should have received a copy of the GNU General Public License
>> >> along
>> >> * with FFmpeg; if not, write to the Free Software Foundation, Inc.,
>> >> * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>> >> + *
>> >> + * Copyright © 2018, VideoLAN and dav1d authors
>> >> + * Copyright © 2018, Two Orioles, LLC
>> >> + * All rights reserved.
>> >> + *
>> >> + * Redistribution and use in source and binary forms, with or without
>> >> + * modification, are permitted provided that the following conditions
>> >> are met:
>> >> + *
>> >> + * 1. Redistributions of source code must retain the above copyright
>> >> notice, this
>> >> + * list of conditions and the following disclaimer.
>> >> + *
>> >> + * 2. Redistributions in binary form must reproduce the above copyright
>> >> notice,
>> >> + * this list of conditions and the following disclaimer in the
>> >> documentation
>> >> + * and/or other materials provided with the distribution.
>> >> + *
>> >> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>> >> "AS IS" AND
>> >> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>> >> IMPLIED
>> >> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
>> >> + * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
>> >> LIABLE FOR
>> >> + * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>> >> CONSEQUENTIAL DAMAGES
>> >> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
>> >> SERVICES;
>> >> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
>> >> CAUSED AND
>> >> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
>> >> TORT
>> >> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>> >> OF THIS
>> >> + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> >> */
>> >
>> > Do we need this double licensing?
>> >
>> >>
>> >> #include "config.h"
>> >> @@ -575,6 +600,16 @@ static int measure_nop_time(void)
>> >> return nop_sum / 500;
>> >> }
>> >>
>> >> +static inline double avg_cycles_per_call(const CheckasmPerf *const p)
>> >> +{
>> >> + if (p->iterations) {
>> >> + const double cycles = (double)(10 * p->cycles) / p->iterations -
>> >> state.nop_time;
>> >> + if (cycles > 0.0)
>> >> + return cycles / 4.0; /* 4 calls per iteration */
>> >> + }
>> >> + return 0.0;
>> >> +}
>> >> +
>> >> /* Print benchmark results */
>> >> static void print_benchs(CheckasmFunc *f)
>> >> {
>> >> @@ -584,17 +619,25 @@ static void print_benchs(CheckasmFunc *f)
>> >> /* Only print functions with at least one assembly version */
>> >> if (f->versions.cpu || f->versions.next) {
>> >> CheckasmFuncVersion *v = &f->versions;
>> >> + const CheckasmPerf *p = &v->perf;
>> >> + const double baseline = avg_cycles_per_call(p);
>> >> + double decicycles;
>> >> do {
>> >> - CheckasmPerf *p = &v->perf;
>> >> if (p->iterations) {
>> >> - int decicycles = (10*p->cycles/p->iterations -
>> >> state.nop_time) / 4;
>> >> + p = &v->perf;
>> >> + decicycles = avg_cycles_per_call(p);
>> >> if (state.csv || state.tsv) {
>> >> const char sep = state.csv ? ',' : '\t';
>> >> - printf("%s%c%s%c%d.%d\n", f->name, sep,
>> >> + printf("%s%c%s%c%.1f\n", f->name, sep,
>> >> cpu_suffix(v->cpu), sep,
>> >> - decicycles / 10, decicycles % 10);
>> >> + decicycles / 10.0);
>> >> } else {
>> >> - printf("%s_%s: %d.%d\n", f->name,
>> >> cpu_suffix(v->cpu), decicycles/10, decicycles%10);
>> >> + const int pad_length = 10 + 50 -
>> >> + printf("%s_%s:", f->name,
>> >> cpu_suffix(v->cpu));
>> >> + const double ratio = decicycles ?
>> >> + baseline / decicycles : 0.0;
>> >> + printf("%*.1f (%5.2fx)\n", FFMAX(pad_length, 0),
>> >> + decicycles / 10.0, ratio);
>> >
>> > It looks a bit noisy to always print "( 1.00x)" for the C versions.
>> > Perhaps it could be left empty or print "ref".
>> >
>> > Also do you plan on improving it even further? For example printing a
>> > table like this:
>> > c sse avx
>> > blockdsp.clear_block: 34.0 3.5 ( 9.59x) 1.8 (18.92x)
>>
>> I would like to leave it as is initially. It can always be changed later
>> though the intention here is to sync it up with dav1d's checkasm.
>
> Should we aim to first improve dav1d's checkasm and then merge the
> changes back in FFmpeg?
Don't think so, perfect is the enemy of better. We can improve it further
afterwards.
-
jd
_______________________________________________
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".
