Re: [Qemu-devel] [PATCH 1/7 v2] target-i386: Eliminate CONFIG_KVM #ifdefs

[Eduardo Habkost]

On Tue, Dec 10, 2013 at 05:36:10PM -0800, Richard Henderson wrote:
> On 12/10/2013 10:55 AM, Eduardo Habkost wrote:
> > The compiler is capable of eliminating the KVM-specific function calls
> > as long as the calling function has an assert(kvm_enabled()) line, so we
> > don't need to wrap all KVM-specific code inside #ifdefs.
> 
> Really?  In tcg/tcg.h we force NDEBUG if not CONFIG_TCG_DEBUG, which makes
> assert expand to nothing.  This statement may be true for some files, but
> almost everything under target-i386 includes tcg.h.
> 
> Although I know we've talked within glibc and gcc the de-optimization of
> missing out on assert info, and how we ought to use __builtin_gcc_unreachable
> in order to retain that, we've still not done anything official with 
> <assert.h>.
> 
> That said, I don't disagree with the changes, if they work with a forced
> -DNDEBUG, i.e. unreachable code that still compiles.

Oops, I didn't consider -DNDEBUG. You are right and assert() can't be
considered a sufficient hint to the compiler.

I remember making some tests where assert(kvm_enabled()) helped the
compiler, but I can't reproduce it anymore (maybe it was in an older
version of the code). I have just tested this patch using -DNEBUG
combined with: -O, -O0, -O1, -O2, and without -O. In all cases, QEMU was
built successfully with CONFIG_KVM disabled.

I will resubmit the patch without the assert() line and with a more
accurate commit message.

-- 
Eduardo