From mboxrd@z Thu Jan 1 00:00:00 1970 From: Richard Henderson Subject: Re: [Qemu-devel] [PATCH for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid() Date: Wed, 13 Nov 2013 12:27:10 +1000 Message-ID: <5282E37E.6090704@twiddle.net> References: <5281580D.7060305@redhat.com> <52816422.8060002@redhat.com> <52821A62.2050001@redhat.com> <52822958.8060508@redhat.com> <20131112132337.GB2008@redhat.com> <528233DD.6020306@redhat.com> <20131112140935.GD2008@redhat.com> <528241E0.5060503@redhat.com> <52824766.2000507@redhat.com> <52825009.10506@redhat.com> <5282794D.20108@twiddle.net> <5282B17B.7020602@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Anthony Liguori , Peter Maydell , kvm-devel , Gleb Natapov , Patch Tracking , QEMU Developers , Andreas Tobler , Anthony Liguori To: Paolo Bonzini Return-path: Received: from mail-qe0-f45.google.com ([209.85.128.45]:57225 "EHLO mail-qe0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755513Ab3KMC2h (ORCPT ); Tue, 12 Nov 2013 21:28:37 -0500 Received: by mail-qe0-f45.google.com with SMTP id 8so6344830qea.32 for ; Tue, 12 Nov 2013 18:28:37 -0800 (PST) In-Reply-To: <5282B17B.7020602@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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__. r~ From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40651) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VgQCE-0001HT-Ne for qemu-devel@nongnu.org; Tue, 12 Nov 2013 21:28:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VgQC6-00087P-Ag for qemu-devel@nongnu.org; Tue, 12 Nov 2013 21:28:46 -0500 Received: from mail-qa0-x234.google.com ([2607:f8b0:400d:c00::234]:39364) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VgQC6-00086C-60 for qemu-devel@nongnu.org; Tue, 12 Nov 2013 21:28:38 -0500 Received: by mail-qa0-f52.google.com with SMTP id ii20so102225qab.4 for ; Tue, 12 Nov 2013 18:28:37 -0800 (PST) Sender: Richard Henderson Message-ID: <5282E37E.6090704@twiddle.net> Date: Wed, 13 Nov 2013 12:27:10 +1000 From: Richard Henderson MIME-Version: 1.0 References: <5281580D.7060305@redhat.com> <52816422.8060002@redhat.com> <52821A62.2050001@redhat.com> <52822958.8060508@redhat.com> <20131112132337.GB2008@redhat.com> <528233DD.6020306@redhat.com> <20131112140935.GD2008@redhat.com> <528241E0.5060503@redhat.com> <52824766.2000507@redhat.com> <52825009.10506@redhat.com> <5282794D.20108@twiddle.net> <5282B17B.7020602@redhat.com> In-Reply-To: <5282B17B.7020602@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Peter Maydell , kvm-devel , Gleb Natapov , Patch Tracking , QEMU Developers , Andreas Tobler , Anthony Liguori , Anthony Liguori 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__. r~