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 <cpuid.h> >>> - >>> +#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 > > >
