Message ID | 528139CB.7080502@FreeBSD.org |
---|---|
State | New |
Headers | show |
On 11 November 2013 20:10, Andreas Tobler <andreast@freebsd.org> wrote: > Paolo asked me to test and submit the below patch to fix compilation and > link with clang. > > Paolo reduced the issue to a clang bug where dead code is not properly > eliminated before linktime. (the clang bug ID: 17882) Thanks for the patch. However, it looks a bit odd to me. Can you quote the error message clang produces, please? I think I would agree with the commenter in the bug report you reference (http://llvm.org/bugs/show_bug.cgi?id=17882) that this is not a clang bug. We shouldn't be relying on the compiler's dead code elimination to get rid of references to functions that don't exist in certain configurations. This will always be unreliable (especially if compiling without optimization). Instead we should either be using ifdefs or stub functions (probably the latter in this case). If you put a stub implementation of kvm_arch_get_supported_cpuid() into target-i386/kvm-stub.c does this fix the compilation issue? thanks -- PMM
On 11.11.13 22:02, Peter Maydell wrote: > On 11 November 2013 20:10, Andreas Tobler <andreast@freebsd.org> wrote: >> Paolo asked me to test and submit the below patch to fix compilation and >> link with clang. >> >> Paolo reduced the issue to a clang bug where dead code is not properly >> eliminated before linktime. (the clang bug ID: 17882) > > Thanks for the patch. However, it looks a bit odd to me. Can > you quote the error message clang produces, please? [tcx58:build/qemu/objdir] andreast% gmake CC x86_64-softmmu/target-i386/cpu.o LINK x86_64-softmmu/qemu-system-x86_64 target-i386/cpu.o: In function `cpu_x86_cpuid': /export/devel/net/src/qemu/qemu-master/target-i386/cpu.c:2203: undefined reference to `kvm_arch_get_supported_cpuid' /export/devel/net/src/qemu/qemu-master/target-i386/cpu.c:2204: undefined reference to `kvm_arch_get_supported_cpuid' /export/devel/net/src/qemu/qemu-master/target-i386/cpu.c:2223: undefined reference to `kvm_arch_get_supported_cpuid' cc: error: linker command failed with exit code 1 (use -v to see invocation) gmake[1]: *** [qemu-system-x86_64] Error 1 gmake: *** [subdir-x86_64-softmmu] Error 2 > I think I would agree with the commenter in the bug report you > reference (http://llvm.org/bugs/show_bug.cgi?id=17882) > that this is not a clang bug. We shouldn't be relying on the > compiler's dead code elimination to get rid of references to > functions that don't exist in certain configurations. This will > always be unreliable (especially if compiling without optimization). > Instead we should either be using ifdefs or stub functions (probably > the latter in this case). I know it is a difficult business. And probably you're right, but from a dump users point of view I do not agree. I'm used to gcc, which is able to compile this, and I expect other compilers to be able to do the same. The compiler should work for me not vice versa :) > If you put a stub implementation of kvm_arch_get_supported_cpuid() > into target-i386/kvm-stub.c does this fix the compilation issue? Will try and let you know. Thanks, Andreas
On 11 November 2013 21:12, Andreas Tobler <andreast@freebsd.org> wrote: > On 11.11.13 22:02, Peter Maydell wrote: >> I think I would agree with the commenter in the bug report you >> reference (http://llvm.org/bugs/show_bug.cgi?id=17882) >> that this is not a clang bug. We shouldn't be relying on the >> compiler's dead code elimination to get rid of references to >> functions that don't exist in certain configurations. This will >> always be unreliable (especially if compiling without optimization). >> Instead we should either be using ifdefs or stub functions (probably >> the latter in this case). > > I know it is a difficult business. And probably you're right, but from a > dump users point of view I do not agree. > > I'm used to gcc, which is able to compile this, and I expect other > compilers to be able to do the same. The compiler should work for me not > vice versa :) Unfortunately C doesn't work that way. If you rely on behaviour which isn't guaranteed by the compiler authors then you have unreliable programs. I bet if you asked the gcc developers they'd say they didn't guarantee this to work either. >> If you put a stub implementation of kvm_arch_get_supported_cpuid() >> into target-i386/kvm-stub.c does this fix the compilation issue? > > Will try and let you know. I've actually reproduced this with my clang/macos system (it was masked by another build failure for which there's a patch on list that hasn't been applied yet). I'm working up a patch that I'll send shortly. thanks -- PMM
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 864c80e..6d3e5fd 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2196,7 +2196,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *ebx = 0; *ecx = 0; *edx = 0; - if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE) || !kvm_enabled()) { + if (!kvm_enabled() || !(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) { break; } kvm_mask =
Hello, Paolo asked me to test and submit the below patch to fix compilation and link with clang. Paolo reduced the issue to a clang bug where dead code is not properly eliminated before linktime. (the clang bug ID: 17882) Thanks, Andreas Signed-off-by: Andreas Tobler <address@hidden>