checkasm: vp9dsp: Benchmark the dc-only version of idct_idct separately
diff mbox

Message ID 1479159974-28495-1-git-send-email-martin@martin.st
State Committed
Commit 81d7f0bbca837afda1f7e60d3ae52ab1360ab44b
Headers show

Commit Message

Martin Storsjö Nov. 14, 2016, 9:46 p.m. 
The dc-only mode is already checked to work correctly above, but this
allows benchmarking this mode for performance tuning, and allows making
sure that it actually is correctly hooked up.
---
 tests/checkasm/vp9dsp.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Luca Barbato Nov. 15, 2016, 9:53 a.m. | #1 
On 14/11/2016 22:46, Martin Storsjö wrote:
> The dc-only mode is already checked to work correctly above, but this
> allows benchmarking this mode for performance tuning, and allows making
> sure that it actually is correctly hooked up.
> ---
>  tests/checkasm/vp9dsp.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tests/checkasm/vp9dsp.c b/tests/checkasm/vp9dsp.c
> index 690e0cf..b9d1c73 100644
> --- a/tests/checkasm/vp9dsp.c
> +++ b/tests/checkasm/vp9dsp.c
> @@ -297,6 +297,12 @@ static void check_itxfm(void)
>                  }
>                  bench_new(dst, sz * SIZEOF_PIXEL, coef, sz * sz);
>              }
> +            if (txtp == 0 && tx != 4) {
> +                if (check_func(dsp.itxfm_add[tx][txtp], "vp9_inv_%s_%dx%d_dc_add",
> +                               txtp_types[txtp], sz, sz)) {
> +                    bench_new(dst, sz * SIZEOF_PIXEL, coef, 1);
> +                }
> +            }
>          }
>      }
>      report("itxfm");
> 

Probably Ok.
Janne Grunau Nov. 15, 2016, 9:35 p.m. | #2 
On 2016-11-14 23:46:14 +0200, Martin Storsjö wrote:
> The dc-only mode is already checked to work correctly above, but this
> allows benchmarking this mode for performance tuning, and allows making
> sure that it actually is correctly hooked up.
> ---
>  tests/checkasm/vp9dsp.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tests/checkasm/vp9dsp.c b/tests/checkasm/vp9dsp.c
> index 690e0cf..b9d1c73 100644
> --- a/tests/checkasm/vp9dsp.c
> +++ b/tests/checkasm/vp9dsp.c
> @@ -297,6 +297,12 @@ static void check_itxfm(void)
>                  }
>                  bench_new(dst, sz * SIZEOF_PIXEL, coef, sz * sz);
>              }
> +            if (txtp == 0 && tx != 4) {
> +                if (check_func(dsp.itxfm_add[tx][txtp], "vp9_inv_%s_%dx%d_dc_add",
> +                               txtp_types[txtp], sz, sz)) {
> +                    bench_new(dst, sz * SIZEOF_PIXEL, coef, 1);
> +                }
> +            }
>          }
>      }
>      report("itxfm");

ok

Janne
Ronald Bultje Nov. 17, 2016, 12:23 p.m. | #3 
Hi,

On Mon, Nov 14, 2016 at 4:46 PM, Martin Storsjö <martin@martin.st> wrote:

> The dc-only mode is already checked to work correctly above, but this
> allows benchmarking this mode for performance tuning, and allows making
> sure that it actually is correctly hooked up.
> ---
>  tests/checkasm/vp9dsp.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tests/checkasm/vp9dsp.c b/tests/checkasm/vp9dsp.c
> index 690e0cf..b9d1c73 100644
> --- a/tests/checkasm/vp9dsp.c
> +++ b/tests/checkasm/vp9dsp.c
> @@ -297,6 +297,12 @@ static void check_itxfm(void)
>                  }
>                  bench_new(dst, sz * SIZEOF_PIXEL, coef, sz * sz);
>              }
> +            if (txtp == 0 && tx != 4) {
> +                if (check_func(dsp.itxfm_add[tx][txtp],
> "vp9_inv_%s_%dx%d_dc_add",
> +                               txtp_types[txtp], sz, sz)) {
> +                    bench_new(dst, sz * SIZEOF_PIXEL, coef, 1);
> +                }
> +            }
>          }
>      }
>      report("itxfm");
> --
> 2.7.4


I had a different local modification that allows tuning all the relevant
sub-IDCTs, basically re-arranging the loops so check_func is inside the
sub-IDCT loop and we bench each sub-IDCT separately. That's more generic
and probably more useful.

Ronald
Martin Storsjö Nov. 17, 2016, 12:37 p.m. | #4 
On Thu, 17 Nov 2016, Ronald S. Bultje wrote:

> Hi,
>
> On Mon, Nov 14, 2016 at 4:46 PM, Martin Storsjö <martin@martin.st> wrote:
>
>> The dc-only mode is already checked to work correctly above, but this
>> allows benchmarking this mode for performance tuning, and allows making
>> sure that it actually is correctly hooked up.
>> ---
>>  tests/checkasm/vp9dsp.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/tests/checkasm/vp9dsp.c b/tests/checkasm/vp9dsp.c
>> index 690e0cf..b9d1c73 100644
>> --- a/tests/checkasm/vp9dsp.c
>> +++ b/tests/checkasm/vp9dsp.c
>> @@ -297,6 +297,12 @@ static void check_itxfm(void)
>>                  }
>>                  bench_new(dst, sz * SIZEOF_PIXEL, coef, sz * sz);
>>              }
>> +            if (txtp == 0 && tx != 4) {
>> +                if (check_func(dsp.itxfm_add[tx][txtp],
>> "vp9_inv_%s_%dx%d_dc_add",
>> +                               txtp_types[txtp], sz, sz)) {
>> +                    bench_new(dst, sz * SIZEOF_PIXEL, coef, 1);
>> +                }
>> +            }
>>          }
>>      }
>>      report("itxfm");
>> --
>> 2.7.4
>
>
> I had a different local modification that allows tuning all the relevant
> sub-IDCTs, basically re-arranging the loops so check_func is inside the
> sub-IDCT loop and we bench each sub-IDCT separately. That's more generic
> and probably more useful.

Right, that's probably more useful. Would you care to finish that 
modification to get it upstreamed in either project?

// Martin
Ronald Bultje Nov. 17, 2016, 8:59 p.m. | #5 
Hi,

On Thu, Nov 17, 2016 at 7:37 AM, Martin Storsjö <martin@martin.st> wrote:

> On Thu, 17 Nov 2016, Ronald S. Bultje wrote:
>
> Hi,
>>
>> On Mon, Nov 14, 2016 at 4:46 PM, Martin Storsjö <martin@martin.st> wrote:
>>
>> The dc-only mode is already checked to work correctly above, but this
>>> allows benchmarking this mode for performance tuning, and allows making
>>> sure that it actually is correctly hooked up.
>>> ---
>>>  tests/checkasm/vp9dsp.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/tests/checkasm/vp9dsp.c b/tests/checkasm/vp9dsp.c
>>> index 690e0cf..b9d1c73 100644
>>> --- a/tests/checkasm/vp9dsp.c
>>> +++ b/tests/checkasm/vp9dsp.c
>>> @@ -297,6 +297,12 @@ static void check_itxfm(void)
>>>                  }
>>>                  bench_new(dst, sz * SIZEOF_PIXEL, coef, sz * sz);
>>>              }
>>> +            if (txtp == 0 && tx != 4) {
>>> +                if (check_func(dsp.itxfm_add[tx][txtp],
>>> "vp9_inv_%s_%dx%d_dc_add",
>>> +                               txtp_types[txtp], sz, sz)) {
>>> +                    bench_new(dst, sz * SIZEOF_PIXEL, coef, 1);
>>> +                }
>>> +            }
>>>          }
>>>      }
>>>      report("itxfm");
>>> --
>>> 2.7.4
>>>
>>
>>
>> I had a different local modification that allows tuning all the relevant
>> sub-IDCTs, basically re-arranging the loops so check_func is inside the
>> sub-IDCT loop and we bench each sub-IDCT separately. That's more generic
>> and probably more useful.
>>
>
> Right, that's probably more useful. Would you care to finish that
> modification to get it upstreamed in either project?


Sure, no problem.

Ronald

Patch
diff mbox 

diff --git a/tests/checkasm/vp9dsp.c b/tests/checkasm/vp9dsp.c
index 690e0cf..b9d1c73 100644
--- a/tests/checkasm/vp9dsp.c
+++ b/tests/checkasm/vp9dsp.c
@@ -297,6 +297,12 @@  static void check_itxfm(void)
                 }
                 bench_new(dst, sz * SIZEOF_PIXEL, coef, sz * sz);
             }
+            if (txtp == 0 && tx != 4) {
+                if (check_func(dsp.itxfm_add[tx][txtp], "vp9_inv_%s_%dx%d_dc_add",
+                               txtp_types[txtp], sz, sz)) {
+                    bench_new(dst, sz * SIZEOF_PIXEL, coef, 1);
+                }
+            }
         }
     }
     report("itxfm");