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

2013-11-13 Thread Peter Maydell
On 13 November 2013 07:25, Paolo Bonzini pbonz...@redhat.com wrote: Il 13/11/2013 03:27, Richard Henderson ha scritto: 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

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

2013-11-12 Thread Peter Maydell
On 11 November 2013 23:21, Anthony Liguori anth...@codemonkey.ws wrote: We're not talking about something obscure here. It's eliminating an if(0) block. No, we're not talking about a simple if (0) expression. What we had in this case was if (!(env-features[FEAT_1_ECX] CPUID_EXT_XSAVE) ||

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

2013-11-12 Thread Paolo Bonzini
Il 12/11/2013 12:07, Peter Maydell ha scritto: On 11 November 2013 23:21, Anthony Liguori anth...@codemonkey.ws wrote: We're not talking about something obscure here. It's eliminating an if(0) block. No, we're not talking about a simple if (0) expression. What we had in this case was if

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

2013-11-12 Thread Peter Maydell
On 12 November 2013 12:09, Paolo Bonzini pbonz...@redhat.com wrote: Il 12/11/2013 12:07, Peter Maydell ha scritto: For the compiler to eliminate this we are relying on: * dead-code elimination of code following a 'break' statement in a case block * constant-folding of something || 1 to 1

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

2013-11-12 Thread Paolo Bonzini
Il 12/11/2013 13:16, Peter Maydell ha scritto: On 12 November 2013 12:09, Paolo Bonzini pbonz...@redhat.com wrote: Il 12/11/2013 12:07, Peter Maydell ha scritto: For the compiler to eliminate this we are relying on: * dead-code elimination of code following a 'break' statement in a case

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

2013-11-12 Thread Peter Maydell
On 12 November 2013 13:12, Paolo Bonzini pbonz...@redhat.com wrote: I'm saying it's *reasonable* to expect that -O0 means reduce compile time, make debugging produce expected results, and try (not too hard) to not break what works at -O2. And that's what we've got. There's no requirement, even

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

2013-11-12 Thread Gleb Natapov
On Tue, Nov 12, 2013 at 02:12:56PM +0100, Paolo Bonzini wrote: Il 12/11/2013 13:16, Peter Maydell ha scritto: On 12 November 2013 12:09, Paolo Bonzini pbonz...@redhat.com wrote: Il 12/11/2013 12:07, Peter Maydell ha scritto: For the compiler to eliminate this we are relying on: *

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

2013-11-12 Thread Gleb Natapov
On Tue, Nov 12, 2013 at 01:21:51PM +, Peter Maydell wrote: (Similarly, you can put code that's a syntax error inside #if 0, but that won't work inside an if (0). The solution is not to do that.) That's the advantage of using if (0) instead of

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

2013-11-12 Thread Paolo Bonzini
Il 12/11/2013 14:23, Gleb Natapov ha scritto: If -O0 does not do that, let's move debug builds to -O1. Why not enable dce with -fdce? First, because clang doesn't have fine-tuned optimization options (at least I couldn't find them and -fdce doesn't work). Second, because most optimization

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

2013-11-12 Thread Peter Maydell
On 12 November 2013 13:12, Paolo Bonzini pbonz...@redhat.com wrote: If -O0 does not do that, let's move debug builds to -O1. Isn't this going to sacrifice debuggability? That also seems like the wrong tradeoff to me. -- PMM

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

2013-11-12 Thread Gleb Natapov
On Tue, Nov 12, 2013 at 02:57:49PM +0100, Paolo Bonzini wrote: Il 12/11/2013 14:23, Gleb Natapov ha scritto: If -O0 does not do that, let's move debug builds to -O1. Why not enable dce with -fdce? First, because clang doesn't have fine-tuned optimization options (at least I couldn't

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

2013-11-12 Thread Peter Maydell
On 12 November 2013 14:09, Gleb Natapov g...@redhat.com wrote: On Tue, Nov 12, 2013 at 02:57:49PM +0100, Paolo Bonzini wrote: Il 12/11/2013 14:23, Gleb Natapov ha scritto: If -O0 does not do that, let's move debug builds to -O1. Why not enable dce with -fdce? First, because clang doesn't

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

2013-11-12 Thread Paolo Bonzini
Il 12/11/2013 15:09, Gleb Natapov ha scritto: On Tue, Nov 12, 2013 at 02:57:49PM +0100, Paolo Bonzini wrote: Il 12/11/2013 14:23, Gleb Natapov ha scritto: If -O0 does not do that, let's move debug builds to -O1. Why not enable dce with -fdce? First, because clang doesn't have fine-tuned

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

2013-11-12 Thread Peter Maydell
On 12 November 2013 14:57, Paolo Bonzini pbonz...@redhat.com wrote: Il 12/11/2013 15:09, Gleb Natapov ha scritto: On Tue, Nov 12, 2013 at 02:57:49PM +0100, Paolo Bonzini wrote: Il 12/11/2013 14:23, Gleb Natapov ha scritto: If -O0 does not do that, let's move debug builds to -O1. Why not

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

2013-11-12 Thread Paolo Bonzini
Il 12/11/2013 16:13, Peter Maydell ha scritto: -O1 then for clang. We can even test in configure for the exact optimizations we want, in fact. But I think -O1 doesn't sacrifice debuggability that much: I'm afraid I still don't see why you'd want to sacrifice it at all, Is this FUD or

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

2013-11-12 Thread Peter Maydell
On 12 November 2013 15:21, Paolo Bonzini pbonz...@redhat.com wrote: Il 12/11/2013 16:13, Peter Maydell ha scritto: -O1 then for clang. We can even test in configure for the exact optimizations we want, in fact. But I think -O1 doesn't sacrifice debuggability that much: I'm afraid I

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

2013-11-12 Thread Paolo Bonzini
Il 12/11/2013 16:32, Peter Maydell ha scritto: Is this FUD or do you have examples of bad debuggability of -O1 code? The clang manpage says specifically Note that Clang debug information works best at -O0. , and I see no reason to disbelieve it. In particular, they don't say we definitely

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

2013-11-12 Thread Peter Maydell
On 12 November 2013 15:58, Paolo Bonzini pbonz...@redhat.com wrote: I don't really see a reason why QEMU should give clang more weight than Windows or Mac OS X. I'm not asking for more weight (and actually my main reason for caring about clang is exactly MacOSX). I'm just asking that when a bug

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

2013-11-12 Thread Anthony Liguori
On Tue, Nov 12, 2013 at 8:08 AM, Peter Maydell peter.mayd...@linaro.org wrote: On 12 November 2013 15:58, Paolo Bonzini pbonz...@redhat.com wrote: I don't really see a reason why QEMU should give clang more weight than Windows or Mac OS X. I'm not asking for more weight (and actually my main

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

2013-11-12 Thread Peter Maydell
On 12 November 2013 17:04, Anthony Liguori anth...@codemonkey.ws wrote: QEMU has always been intimately tied to GCC. Heck, it all started as a giant GCC hack relying on entirely undocumented behavior (dyngen's disassembly of functions). It has historically. Blue Swirl put in a lot of work to

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

2013-11-12 Thread Richard Henderson
On 11/13/2013 03:04 AM, Anthony Liguori wrote: On Tue, Nov 12, 2013 at 8:08 AM, Peter Maydell peter.mayd...@linaro.org wrote: On 12 November 2013 15:58, Paolo Bonzini pbonz...@redhat.com wrote: I don't really see a reason why QEMU should give clang more weight than Windows or Mac OS X. I'm

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

2013-11-12 Thread Peter Maydell
On 12 November 2013 18:54, Richard Henderson r...@twiddle.net wrote: 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. I think that makes sense and would be happy with

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

2013-11-12 Thread Stefan Weil
Am 12.11.2013 19:57, schrieb Peter Maydell: On 12 November 2013 18:54, Richard Henderson r...@twiddle.net wrote: 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. I

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

2013-11-12 Thread Paolo Bonzini
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

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

2013-11-12 Thread Richard Henderson
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

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

2013-11-12 Thread Paolo Bonzini
Il 13/11/2013 03:27, Richard Henderson ha scritto: 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

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

2013-11-12 Thread 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)

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

2013-11-11 Thread Peter Maydell
Fix build failures with clang when KVM is not enabled by providing a stub version of kvm_arch_get_supported_cpuid(). Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- I wouldn't be surprised if this also affected debug gcc builds with KVM disabled, but I haven't checked. Incidentally,

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

2013-11-11 Thread Andreas Tobler
On 11.11.13 22:22, Peter Maydell wrote: Fix build failures with clang when KVM is not enabled by providing a stub version of kvm_arch_get_supported_cpuid(). Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- I wouldn't be surprised if this also affected debug gcc builds with KVM

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

2013-11-11 Thread Paolo Bonzini
Il 11/11/2013 22:22, Peter Maydell ha scritto: Fix build failures with clang when KVM is not enabled by providing a stub version of kvm_arch_get_supported_cpuid(). Signed-off-by: Peter Maydell peter.mayd...@linaro.org No, please don't. We are already relying on dead code elimination for KVM

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

2013-11-11 Thread Peter Maydell
On 11 November 2013 22:19, Paolo Bonzini pbonz...@redhat.com wrote: Il 11/11/2013 22:22, Peter Maydell ha scritto: Fix build failures with clang when KVM is not enabled by providing a stub version of kvm_arch_get_supported_cpuid(). Signed-off-by: Peter Maydell peter.mayd...@linaro.org No,

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

2013-11-11 Thread Paolo Bonzini
Il 11/11/2013 23:38, Peter Maydell ha scritto: If we have other places where we're relying on dead code elimination to not provide a function definition, please point them out, because they're bugs we need to fix, ideally before they cause compilation failures. I'm not sure, there are

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

2013-11-11 Thread Anthony Liguori
On Mon, Nov 11, 2013 at 3:11 PM, Paolo Bonzini pbonz...@redhat.com wrote: Il 11/11/2013 23:38, Peter Maydell ha scritto: If we have other places where we're relying on dead code elimination to not provide a function definition, please point them out, because they're bugs we need to fix,

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

2013-11-11 Thread Peter Maydell
On 11 November 2013 23:11, Paolo Bonzini pbonz...@redhat.com wrote: Il 11/11/2013 23:38, Peter Maydell ha scritto: If we have other places where we're relying on dead code elimination to not provide a function definition, please point them out, because they're bugs we need to fix, ideally

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

2013-11-11 Thread Paolo Bonzini
Il 12/11/2013 00:21, Anthony Liguori ha scritto: FWIW, I'd rather just add -O1 for debug builds than add more stub functions. That can do, too. clang works fine with -O1. Paolo