diff mbox series

[RESEND,v2,1/2] migration/xbzrle: use ctz64 to avoid undefined result

Message ID 117f3c2fdc17c319b0e04014bbd7e0c94992c197.1678733663.git.quic_mathbern@quicinc.com
State New
Headers show
Series migration/xbzrle: fix two avx512 runtime issues | expand

Commit Message

Matheus Tavares Bernardino March 13, 2023, 6:58 p.m. UTC
__builtin_ctzll() produces undefined results when the argument is 0.
This can be seen through test-xbzrle, which produces the following
warning:

../migration/xbzrle.c:265: runtime error: passing zero to ctz(), which is not a valid argument

Replace __builtin_ctzll() with our ctz64() wrapper which properly
handles 0.

Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
---
 migration/xbzrle.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Dr. David Alan Gilbert March 15, 2023, 6:01 p.m. UTC | #1
* Matheus Tavares Bernardino (quic_mathbern@quicinc.com) wrote:
> __builtin_ctzll() produces undefined results when the argument is 0.
> This can be seen through test-xbzrle, which produces the following
> warning:
> 
> ../migration/xbzrle.c:265: runtime error: passing zero to ctz(), which is not a valid argument
> 
> Replace __builtin_ctzll() with our ctz64() wrapper which properly
> handles 0.
> 
> Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  migration/xbzrle.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/xbzrle.c b/migration/xbzrle.c
> index 05366e86c0..21b92d4eae 100644
> --- a/migration/xbzrle.c
> +++ b/migration/xbzrle.c
> @@ -12,6 +12,7 @@
>   */
>  #include "qemu/osdep.h"
>  #include "qemu/cutils.h"
> +#include "qemu/host-utils.h"
>  #include "xbzrle.h"
>  
>  /*
> @@ -233,7 +234,7 @@ int xbzrle_encode_buffer_avx512(uint8_t *old_buf, uint8_t *new_buf, int slen,
>                      break;
>                  }
>                  never_same = false;
> -                num = __builtin_ctzll(~comp);
> +                num = ctz64(~comp);
>                  num = (num < bytes_to_check) ? num : bytes_to_check;
>                  zrun_len += num;
>                  bytes_to_check -= num;
> @@ -262,7 +263,7 @@ int xbzrle_encode_buffer_avx512(uint8_t *old_buf, uint8_t *new_buf, int slen,
>                  nzrun_len += 64;
>                  break;
>              }
> -            num = __builtin_ctzll(comp);
> +            num = ctz64(comp);
>              num = (num < bytes_to_check) ? num : bytes_to_check;
>              nzrun_len += num;
>              bytes_to_check -= num;
> -- 
> 2.39.1
>
Juan Quintela March 15, 2023, 8:56 p.m. UTC | #2
Matheus Tavares Bernardino <quic_mathbern@quicinc.com> wrote:
> __builtin_ctzll() produces undefined results when the argument is 0.
> This can be seen through test-xbzrle, which produces the following
> warning:
>
> ../migration/xbzrle.c:265: runtime error: passing zero to ctz(), which is not a valid argument
>
> Replace __builtin_ctzll() with our ctz64() wrapper which properly
> handles 0.
>
> Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

queued.
diff mbox series

Patch

diff --git a/migration/xbzrle.c b/migration/xbzrle.c
index 05366e86c0..21b92d4eae 100644
--- a/migration/xbzrle.c
+++ b/migration/xbzrle.c
@@ -12,6 +12,7 @@ 
  */
 #include "qemu/osdep.h"
 #include "qemu/cutils.h"
+#include "qemu/host-utils.h"
 #include "xbzrle.h"
 
 /*
@@ -233,7 +234,7 @@  int xbzrle_encode_buffer_avx512(uint8_t *old_buf, uint8_t *new_buf, int slen,
                     break;
                 }
                 never_same = false;
-                num = __builtin_ctzll(~comp);
+                num = ctz64(~comp);
                 num = (num < bytes_to_check) ? num : bytes_to_check;
                 zrun_len += num;
                 bytes_to_check -= num;
@@ -262,7 +263,7 @@  int xbzrle_encode_buffer_avx512(uint8_t *old_buf, uint8_t *new_buf, int slen,
                 nzrun_len += 64;
                 break;
             }
-            num = __builtin_ctzll(comp);
+            num = ctz64(comp);
             num = (num < bytes_to_check) ? num : bytes_to_check;
             nzrun_len += num;
             bytes_to_check -= num;