From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45926) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bk619-0007wx-Pf for qemu-devel@nongnu.org; Wed, 14 Sep 2016 04:58:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bk613-0004ls-P4 for qemu-devel@nongnu.org; Wed, 14 Sep 2016 04:58:06 -0400 Received: from mail-wm0-f43.google.com ([74.125.82.43]:38369) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bk613-0004li-Iy for qemu-devel@nongnu.org; Wed, 14 Sep 2016 04:58:01 -0400 Received: by mail-wm0-f43.google.com with SMTP id 1so17237311wmz.1 for ; Wed, 14 Sep 2016 01:58:01 -0700 (PDT) Sender: Paolo Bonzini References: <1473800239-13841-1-git-send-email-rth@twiddle.net> <1f1996f1-1e91-6b2d-2c76-34a592066d76@twiddle.net> From: Paolo Bonzini Message-ID: <6c0a2f11-1799-830e-b947-16878781e667@redhat.com> Date: Wed, 14 Sep 2016 10:56:56 +0200 MIME-Version: 1.0 In-Reply-To: <1f1996f1-1e91-6b2d-2c76-34a592066d76@twiddle.net> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5] cutils: Rewrite x86 buffer zero checking List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson , qemu-devel@nongnu.org On 14/09/2016 03:11, Richard Henderson wrote: > On 09/13/2016 04:21 PM, Paolo Bonzini wrote: >> >> >> On 13/09/2016 22:57, Richard Henderson wrote: >>> -#if defined(CONFIG_AVX2_OPT) || (defined(CONFIG_CPUID_H) && >>> defined(__SSE2__)) >>> -#include >>> - >>> +#if (defined(CONFIG_AVX2_OPT) && defined(CONFIG_CPUID_H)) || >>> defined(__SSE2__) >> >> Your __SSE2__ version is better than mine which required cpuid.h just to >> simplify the logic a bit. On the other hand, CONFIG_CPUID_H is not >> needed in CONFIG_AVX2_OPT, because the test already requires cpuid.h. > > Hmm, it does, although it needn't -- the test case would compile without > it. > > Although I bet there's no situation in which the pragmas are supported > and cpuid.h isn't, I think it's cleaner not to infer stuff like this. Yeah, I agree. But we can change the test to not look at cpuid.h separately. >>> +#ifdef CONFIG_CPUID_H >>> +# define INIT_CACHE >>> +# define INIT_ACCEL >>> +#else >>> +# ifndef __SSE2__ >>> +# error "ISA selection confusion" >>> +# endif >>> +# define INIT_CACHE = CACHE_SSE2 >>> +# define INIT_ACCEL = buffer_zero_sse2 >>> #endif >> >> This is ugly, any reason not to initialize INIT_CACHE/INIT_ACCEL to >> respectively 0 and NULL, or 0 and buffer_zero_int in the #ifdef >> CONFIG_CPUID_H case? > > I was hoping to avoid an extra RELATIVE relocation in the (normal) PIE > case. There would be no relocation for 0 and NULL, right? GCC would actually put them in bss, IIRC. Paolo >>> +#undef INIT_CACHE >>> +#undef INIT_ACCEL >> >> The #undef is not really necessary since this file hardly has anything >> after the toplevel #endif. > > Fair enough. > > > r~ > >> >> Just tell me which changes you agree with, I can make them locally. >> >> Paolo > > >