[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/7 v2] target-i386: Eliminate CONFIG_KVM #ifdef
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH 1/7 v2] target-i386: Eliminate CONFIG_KVM #ifdefs |
Date: |
Wed, 11 Dec 2013 14:11:53 -0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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