Re: [PATCH v2 2/5] benchtests: Add memset zero fill benchtest

[naohirot--- via Libc-alpha <libc-alpha@sourceware.org>]

Hi Wilco,

Thank you for the explanation.

> >> might as well change the outer loop to use a larger 'n'. The accuracy
> >> will be bad unless 'n' is really large, and there is no way to improve it.
> >
> > Umm I couldn't understand the logic of this part.
> > How do we change the the outer loop to use a larger 'n'?
> 
> By removing the * 16 from the inner loop and adding it to the outer loop.
> That avoids the confusion that we are testing size 'n' when we are really
> testing n*16.

There may be miscomminuation.
The * 16 is already in the outer loop (1).
Let me copy the code from the mail [1] I put in the previouse mail [2].

-----
static void
__attribute__((noinline, noclone))
do_one_test (json_ctx_t *json_ctx, impl_t *impl, CHAR *s,
             int c1 __attribute ((unused)), int c2 __attribute ((unused)),
             size_t n)
{
  size_t i, j, iters = 32;
  timing_t start, stop, cur, latency = 0;

  for (i = 0; i < 2; i++)
    {
      __builtin_memset (s, c1, n * 16);  // (1)
      TIMING_NOW (start);
      for (j = 0; j < 16; j++)
        CALL (impl, s + n * j, c2, n);
      TIMING_NOW (stop);
      TIMING_DIFF (cur, start, stop);
      TIMING_ACCUM (latency, cur);
    }

  json_element_double (json_ctx, (double) latency / (double) iters);
}
-----

[1] https://sourceware.org/pipermail/libc-alpha/2021-July/129461.html
[2] https://sourceware.org/pipermail/libc-alpha/2021-July/129462.html

BTW, are you thinking the code like this?
But it must be not, because there is no inner and outer loops.

  for (i = 0; i < 16; i++)
    {
      __builtin_memset (s, c1, n);

      TIMING_NOW (start);
      CALL (impl, s, c2, n);
      TIMING_NOW (stop);
      TIMING_DIFF (cur, start, stop);
      TIMING_ACCUM (latency, cur);
    }

> 
> >  CALL (impl, s, c1, n * 16);
> > to
> >   __builtin_memset (s, c1, n * 16);
> >
> > Is this the array you mentioned?
> 
> That doesn't make any sense since that will just call memset and use the
> default ifunc for memset.

This is my intention, because "CALL (impl, s, c1, n * 16);" is not
measured, that is outside of "TIMING_NOW (start);" and "TIMING_NOW (stop);". 
It doesn't matter what kind of memset is called, but matters the
function name in the code so that we can understand it is not mesured.

> 
> What I mean is something trivial like: CALL (impl, s, memset_array[i & 15], n);
> This way you can test any kind of pattern (like all zero, all one, and combinations
> with varying number of zero->non-zero and non-zero->zero transitions).

I understood, thanks.
Why don't we separate it to another patch if it is really matter?

From AArch64 point of view, the purpose of this bench is to measure
"DC ZVA" performance. So non-zero value can be any value except zero.
Do we have any specific reason to vary the non-zero value?

Thanks.
Naohiro