RE: [PATCH 1/1] Add AVX512 support for xbzrle_encode_buffer function

["Zhao, Zhou" <zhou.zhao@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2479 bytes --]

1 we need gcc machined options to tirgger gcc enable SIMD instructions(https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html).  Or it will trigger a compile error. This is the reason why we change the config file.

[cid:image001.png@01D89DB3.EF0B6D80]


$ gcc -mavx2 -dM -E - < /dev/null | egrep "SSE|AVX" | sort
#define __AVX__ 1
#define __AVX2__ 1
#define __SSE__ 1
#define __SSE2__ 1
#define __SSE2_MATH__ 1
#define __SSE3__ 1
#define __SSE4_1__ 1
#define __SSE4_2__ 1
#define __SSE_MATH__ 1
#define __SSSE3__ 1





2 we are not introducing new intel library actually(intel already contribute our intric code to gcc  https://github.com/gcc-mirror/gcc/blob/master/gcc/config/i386/immintrin.h), we just check if gcc can compile avx512 code during compiling， agree to add a runtime check will be better, let me add some code there



-----Original Message-----
From: Daniel P. Berrangé <berrange@redhat.com>
Sent: Friday, July 22, 2022 12:42 AM
To: Zhao, Zhou <zhou.zhao@intel.com>
Cc: Xu, Ling1 <ling1.xu@intel.com>; qemu-devel@nongnu.org; quintela@redhat.com; dgilbert@redhat.com; Jin, Jun I <jun.i.jin@intel.com>
Subject: Re: [PATCH 1/1] Add AVX512 support for xbzrle_encode_buffer function



On Thu, Jul 21, 2022 at 04:02:49PM +0000, Zhao, Zhou wrote:

> Hi dainel:

>   Cause our code depend on intel intrinsics lib implement. And this

> lib depend on macro like  " AVX512BW ". This macro need compile time

> check to enable some machine options . if you only use that utility to

> do runtime check ,you will met compile issue.

> And also if we want to save cpu time , we'd better check it in compile

> time.



Can you give more info about this library ? The patch isn't linking to any new library so it was not immediately obvious that this patch even introduced a dependancy on an external library. Preferrably the library would be using 'pkg-config'

so we can check for it in a single line of meson.build code instead of writing 100's of lines of configure compile tests.



Generally if the library is available in Linux dijstros, then we'll want the CI to test that too, which will mean updating the dockerfiles.



With regards,

Daniel

--

|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|

|: https://libvirt.org         -o-            https://fstop138.berrange.com :|

|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



[-- Attachment #1.2: Type: text/html, Size: 9319 bytes --]

[-- Attachment #2: image001.png --]
[-- Type: image/png, Size: 104493 bytes --]