Re: [Qemu-devel] [PATCH for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid()

[Gleb Natapov]

On Wed, Nov 13, 2013 at 12:27:10PM +1000, Richard Henderson wrote:
> On 11/13/2013 08:53 AM, Paolo Bonzini wrote:
> > Il 12/11/2013 19:54, Richard Henderson ha scritto:
> >> For what it's worth, I think BOTH of the patches that have been posted
> >> should be applied.  That is, the patch that does (X || 1) -> (1 || X),
> >> and the patch that adds the stub.
> >>
> >> Frankly I'd have thought this was obvious
> > 
> > It's not that obvious to me.
> > 
> > If you add the stub, the patch that reorders operands is not necessary.
> >  If you reorder operands, the stub is not necessary.
> > 
> > The patch that does (X || 1) -> (1 || X) is unnecessary as a
> > microoptimization, since this code basically runs once at startup.  The
> > code is also a little bit less clear with the reordered operands, but
> > perhaps that's just me because I wrote the code that way.  (Splitting
> > the if in two would also make sense, and would not affect clarity).
> > 
> > Why should both be applied?
> 
> It's worth working around the clang missed optimization, if for nothing else
> than avoiding the noise of the bugs that would otherwise be filed against the
> release.
> 
> I think it's also worthwhile to implement the kvm api in kvm-stub.c,
> unnecessary or not.  If you really want compile-time feedback on those that
> ought to have been removed by optimization, you could elide them from the stub
> file depending on ifndef __OPTIMIZE__.
> 
Sounds like a nice compromise.

--
                        Gleb.