[FFmpeg-devel] [PATCH] checkasm: Test whether direct cycle counter access works

[Martin Storsjö]

On Thu, 11 Jan 2024, Rémi Denis-Courmont wrote:

> Le torstaina 11. tammikuuta 2024, 14.53.05 EET Martin Storsjö a écrit :
>> This should print a nicer error message than crashing due to
>> an illegal instruction, if direct cycle counter access isn't
>> allowed.
>> 
>> This matches the dav1d checkasm commit
>> 95a192549a448b70d9542e840c4e34b60d09b093.
>> ---
>>  tests/checkasm/checkasm.c | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
>> index 994d64e96b..9c5abb53dc 100644
>> --- a/tests/checkasm/checkasm.c
>> +++ b/tests/checkasm/checkasm.c
>> @@ -754,6 +754,14 @@ static int bench_init_kperf(void)
>>  static int bench_init_ffmpeg(void)
>>  {
>>  #ifdef AV_READ_TIME
>> +    if (!checkasm_save_context()) {
>> +        checkasm_set_signal_handler_state(1);
>> +        AV_READ_TIME();
>> +        checkasm_set_signal_handler_state(0);
>> +    } else {
>> +        fprintf(stderr, "checkasm: unable to access cycle counter\n");
>
> AV_READ_TIME() reads time, not cycles.

Right, I can adjust the wording. Exactly what kind of measurement 
AV_READ_TIME returns varies between architectures and environments indeed.

What about:

     checkasm: unable to execute platform specific timer

> If we want cycle count, then we should add a separate macro, as the two 
> are different performance counters at least on RISC-V.

That's not what I try to do here, I just want to test whether the timer, 
whatever we have in AV_READ_TIME, is usable.

> As things stand, this code won't do anything on RISC-V, sinec 
> AV_READ_TIME() actually reads, well, time, not cycles.

Should I interpret this, as, the current AV_READ_TIME implementation on 
RISC-V always succeeds, contrary to the previous implementation (with 
rdcycle) which is unavailable on some systems, referencing 
05115a77e012331b6ff5e24bab40e75848447c62?

In that case - sure, this would be mostly a no-op for RISC-V, just like it 
is for x86, but for ARM/AArch64 it would provide a nicer error message if 
access to the relevant registers hasn't been configured.

// Martin