RE: [PATCH v3 2/2] Test code for AVX512 support for xbzrle_encode_buffer

[Xu, Ling1]

Hi, Thomas,
      Thanks for your reply. This test code can only work on system supporting 
avx512. It's reasonably to add condition check in test code to, agree to your 
suggestion. I'll add condition check in test code later. 

Best Regards
Ling

-----Original Message-----
From: Thomas Huth <thuth@redhat.com> 
Sent: Monday, August 8, 2022 4:09 PM
To: Xu, Ling1 <ling1.xu@intel.com>; qemu-devel@nongnu.org
Cc: quintela@redhat.com; dgilbert@redhat.com; Zhao, Zhou <zhou.zhao@intel.com>; 
Jin, Jun I <jun.i.jin@intel.com>
Subject: Re: [PATCH v3 2/2] Test code for AVX512 support for 
xbzrle_encode_buffer

On 08/08/2022 09.48, ling xu wrote:
> Signed-off-by: ling xu <ling1.xu@intel.com>
> Co-authored-by: Zhou Zhao <zhou.zhao@intel.com>
> Co-authored-by: Jun Jin <jun.i.jin@intel.com>
> ---
>   tests/unit/test-xbzrle.c | 307 ++++++++++++++++++++++++++++++++++++---
>   1 file changed, 290 insertions(+), 17 deletions(-)
> 
> diff --git a/tests/unit/test-xbzrle.c b/tests/unit/test-xbzrle.c index 
> ef951b6e54..653016826f 100644
> --- a/tests/unit/test-xbzrle.c
> +++ b/tests/unit/test-xbzrle.c
> @@ -38,111 +38,280 @@ static void test_uleb(void)
>       g_assert(val == 0);
>   }
>   
> -static void test_encode_decode_zero(void)
> +static float *test_encode_decode_zero(void)
>   {
>       uint8_t *buffer = g_malloc0(XBZRLE_PAGE_SIZE);
>       uint8_t *compressed = g_malloc0(XBZRLE_PAGE_SIZE);
> +    uint8_t *buffer512 = g_malloc0(XBZRLE_PAGE_SIZE);
> +    uint8_t *compressed512 = g_malloc0(XBZRLE_PAGE_SIZE);
>       int i = 0;
> -    int dlen = 0;
> +    int dlen = 0, dlen512 = 0;
>       int diff_len = g_test_rand_int_range(0, XBZRLE_PAGE_SIZE - 
> 1006);
>   
>       for (i = diff_len; i > 0; i--) {
>           buffer[1000 + i] = i;
> +        buffer512[1000 + i] = i;
>       }
>   
>       buffer[1000 + diff_len + 3] = 103;
>       buffer[1000 + diff_len + 5] = 105;
>   
> +    buffer512[1000 + diff_len + 3] = 103;
> +    buffer512[1000 + diff_len + 5] = 105;
> +
>       /* encode zero page */
> +    time_t t_start, t_end, t_start512, t_end512;
> +    t_start = clock();
>       dlen = xbzrle_encode_buffer(buffer, buffer, XBZRLE_PAGE_SIZE, 
> compressed,
>                          XBZRLE_PAGE_SIZE);
> +    t_end = clock();
> +    float time_val = difftime(t_end, t_start);
>       g_assert(dlen == 0);
>   
> +    t_start512 = clock();
> +    dlen512 = xbzrle_encode_buffer_512(buffer512, buffer512, 
> XBZRLE_PAGE_SIZE,
> +                                       compressed512, 
> + XBZRLE_PAGE_SIZE);

Does this also still work on systems without AVX? If I've got patch 1/2 right, 
this function is only defined if CONFIG_AVX512BW_OPT has been set, so using it 
unconditionally here seems to be wrong?

  Thomas