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.
-----Original Message----- From: Daniel P. Berrangé <berra...@redhat.com> Sent: Thursday, July 21, 2022 11:11 PM To: Xu, Ling1 <ling1...@intel.com> Cc: qemu-devel@nongnu.org; quint...@redhat.com; dgilb...@redhat.com; Zhao, Zhou <zhou.z...@intel.com>; Jin, Jun I <jun.i....@intel.com> Subject: Re: [PATCH 1/1] Add AVX512 support for xbzrle_encode_buffer function On Thu, Jul 21, 2022 at 06:31:47PM +0800, ling xu wrote: > This commit adds AVX512 implementation of xbzrle_encode_buffer > function to accelerate xbzrle encoding speed. Compared with C version > of xbzrle_encode_buffer function, > AVX512 version can achieve almost 60%-70% performance improvement on unit > test provided by qemu. > In addition, we provide one more unit test called > "test_encode_decode_random", in which dirty data are randomly located in 4K > page, and this case can achieve almost 140% performance gain. > > Signed-off-by: ling xu <ling1...@intel.com> > Co-authored-by: Zhou Zhao <zhou.z...@intel.com> > Co-authored-by: Jun Jin <jun.i....@intel.com> > --- > configure | 434 ++++++++++++++++++++++++++++++++++++++- > migration/ram.c | 6 + > migration/xbzrle.c | 177 ++++++++++++++++ > migration/xbzrle.h | 4 + > tests/unit/test-xbzrle.c | 307 +++++++++++++++++++++++++-- > 5 files changed, 908 insertions(+), 20 deletions(-) > diff --git a/migration/ram.c b/migration/ram.c index > 01f9cc1d72..3b931c325f 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -747,9 +747,15 @@ static int save_xbzrle_page(RAMState *rs, uint8_t > **current_data, > memcpy(XBZRLE.current_buf, *current_data, TARGET_PAGE_SIZE); > > /* XBZRLE encoding (if there is no overflow) */ > + #if defined(__x86_64__) && defined(__AVX512BW__) > + encoded_len = xbzrle_encode_buffer_512(prev_cached_page, > XBZRLE.current_buf, > + TARGET_PAGE_SIZE, XBZRLE.encoded_buf, > + TARGET_PAGE_SIZE); > + #else > encoded_len = xbzrle_encode_buffer(prev_cached_page, XBZRLE.current_buf, > TARGET_PAGE_SIZE, XBZRLE.encoded_buf, > TARGET_PAGE_SIZE); > + #endif Shouldn't we be deciding which impl using a runtime check of the current CPUID, rather than a compile time check ? I'm thinking along the lines of what util/bufferiszero.c does to select different optimized versions based on CPUID. The build host CPU features can't be expected to match the runtime host CPU features. 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 :|
