| Message ID | 1353683570-30525-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
Il 23/11/2012 16:12, Peter Maydell ha scritto: > Adjust the conditional which guards the implementation of > cpu_get_real_ticks() via RDTSC, so that we don't try to use it > on x86 CPUs which don't implement RDTSC. Instead we will fall > back to the no-cycle-counter-available default implementation. > > Reported-by: Yurij Popov <oss@djphoenix.ru> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > qemu-timer.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/qemu-timer.h b/qemu-timer.h > index da7e97c..e35f163 100644 > --- a/qemu-timer.h > +++ b/qemu-timer.h > @@ -169,7 +169,7 @@ static inline int64_t cpu_get_real_ticks(void) > return retval; > } > > -#elif defined(__i386__) > +#elif defined(__i586__) > > static inline int64_t cpu_get_real_ticks(void) > { > You should at least test __i686__ too: $ gcc -m32 -dM -E -x c /dev/null |grep __i #define __i686 1 #define __i686__ 1 #define __i386 1 #define __i386__ 1 Paolo
On 23 November 2012 15:15, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 23/11/2012 16:12, Peter Maydell ha scritto: >> Adjust the conditional which guards the implementation of >> >> -#elif defined(__i386__) >> +#elif defined(__i586__) >> >> static inline int64_t cpu_get_real_ticks(void) >> { >> > > You should at least test __i686__ too: > > $ gcc -m32 -dM -E -x c /dev/null |grep __i > #define __i686 1 > #define __i686__ 1 > #define __i386 1 > #define __i386__ 1 Yuck. I had assumed gcc would define everything from i386 on up when building for later cores. -- PMM
Peter Maydell wrote: > On 23 November 2012 15:15, Paolo Bonzini <pbonzini@redhat.com> wrote: > > Il 23/11/2012 16:12, Peter Maydell ha scritto: > >> Adjust the conditional which guards the implementation of > >> > >> -#elif defined(__i386__) > >> +#elif defined(__i586__) > >> > >> static inline int64_t cpu_get_real_ticks(void) > >> { > >> > > > > You should at least test __i686__ too: > > > > $ gcc -m32 -dM -E -x c /dev/null |grep __i > > #define __i686 1 > > #define __i686__ 1 > > #define __i386 1 > > #define __i386__ 1 > > Yuck. I had assumed gcc would define everything from i386 > on up when building for later cores. No, and it doesn't define __i686__ on all x86-32 targets after i686 either: $ gcc -march=core2 -dM -E -x c /dev/null | grep __[0-9a-z] | sort #define __core2 1 #define __core2__ 1 #define __gnu_linux__ 1 #define __i386 1 #define __i386__ 1 #define __linux 1 #define __linux__ 1 #define __tune_core2__ 1 #define __unix 1 #define __unix__ 1 x86 instruction sets haven't followed a linear progression of features for quite a while, especially including non-Intel chips, so it stopped making sense for GCC to indicate the instruction set in that way. GCC 4.6.3 defines __i586__ only when the target arch is set by -march (or default) to i586, pentium or pentium-mmx. And it defines __i686__ only when -march= is set (or default) to c3-2, i686, pentiumpro, pentium2, pentium3, pentium3m or pentium-m. Otherwise it's just things like __athlon__, __corei7__, etc. The only one that's consistent is __i386__ (and __i386). -- Jamie
On 23 November 2012 15:17, Peter Maydell <peter.maydell@linaro.org> wrote: > On 23 November 2012 15:15, Paolo Bonzini <pbonzini@redhat.com> wrote: >> You should at least test __i686__ too: >> >> $ gcc -m32 -dM -E -x c /dev/null |grep __i >> #define __i686 1 >> #define __i686__ 1 >> #define __i386 1 >> #define __i386__ 1 > > Yuck. I had assumed gcc would define everything from i386 > on up when building for later cores. ...and there's an enormous list of x86 cores too. This bites us already -- if you use '-march=native' to get "best for my cpu" then on a Core2, say, it will define __i386__ and __core2__ but not __i686__, so TCG won't use cmov :-( Anybody got any good ideas for how to say "is this at least a 586/686?" in a way that won't fail for any newly introduced x86 core types? -- PMM
On 23 November 2012 15:31, Jamie Lokier <jamie@shareable.org> wrote: > x86 instruction sets haven't followed a linear progression of features > for quite a while, especially including non-Intel chips, so it stopped > making sense for GCC to indicate the instruction set in that way. If you're going to go down that route you need to start defining #defines for features then, so we could say defined(__rdtsc__) or defined(__cmov__) and so on. I don't see any of those either :-( -- PMM
Peter Maydell wrote: > On 23 November 2012 15:17, Peter Maydell <peter.maydell@linaro.org> wrote: > > On 23 November 2012 15:15, Paolo Bonzini <pbonzini@redhat.com> wrote: > >> You should at least test __i686__ too: > >> > >> $ gcc -m32 -dM -E -x c /dev/null |grep __i > >> #define __i686 1 > >> #define __i686__ 1 > >> #define __i386 1 > >> #define __i386__ 1 > > > > Yuck. I had assumed gcc would define everything from i386 > > on up when building for later cores. > > ...and there's an enormous list of x86 cores too. This bites > us already -- if you use '-march=native' to get "best for my > cpu" then on a Core2, say, it will define __i386__ and __core2__ > but not __i686__, so TCG won't use cmov :-( > > Anybody got any good ideas for how to say "is this at least > a 586/686?" in a way that won't fail for any newly introduced > x86 core types? Fwiw, cmov doesn't work on some VIA "686" class CPUs. Shouldn't TCG decide whether to use cmov at runtime anyway, using cpuid? For dynamically generated code it would seem not very expensive to do that. Looking at GCC source, it has an internal flag to say whether the target has cmov, but doesn't expose it in preprocessor conditionals. -- Jamie
Peter Maydell wrote: > On 23 November 2012 15:31, Jamie Lokier <jamie@shareable.org> wrote: > > x86 instruction sets haven't followed a linear progression of features > > for quite a while, especially including non-Intel chips, so it stopped > > making sense for GCC to indicate the instruction set in that way. > > If you're going to go down that route you need to start defining > #defines for features then, so we could say defined(__rdtsc__) > or defined(__cmov__) and so on. I don't see any of those either :-( It does for some major architectural instructions groups like MMX, different kinds of SSE, etc. But not everything and I don't see cmov among them. I agree it's unfortunate. -- Jamie
On Fri, Nov 23, 2012 at 03:12:50PM +0000, Peter Maydell wrote: > Adjust the conditional which guards the implementation of > cpu_get_real_ticks() via RDTSC, so that we don't try to use it > on x86 CPUs which don't implement RDTSC. Instead we will fall > back to the no-cycle-counter-available default implementation. > > Reported-by: Yurij Popov <oss@djphoenix.ru> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > qemu-timer.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/qemu-timer.h b/qemu-timer.h > index da7e97c..e35f163 100644 > --- a/qemu-timer.h > +++ b/qemu-timer.h > @@ -169,7 +169,7 @@ static inline int64_t cpu_get_real_ticks(void) > return retval; > } > > -#elif defined(__i386__) > +#elif defined(__i586__) > > static inline int64_t cpu_get_real_ticks(void) > { > -- > 1.7.9.5 Dropping this due to the issue with gcc __i586__ that has been discussed. Stefan
diff --git a/qemu-timer.h b/qemu-timer.h index da7e97c..e35f163 100644 --- a/qemu-timer.h +++ b/qemu-timer.h @@ -169,7 +169,7 @@ static inline int64_t cpu_get_real_ticks(void) return retval; } -#elif defined(__i386__) +#elif defined(__i586__) static inline int64_t cpu_get_real_ticks(void) {
Adjust the conditional which guards the implementation of cpu_get_real_ticks() via RDTSC, so that we don't try to use it on x86 CPUs which don't implement RDTSC. Instead we will fall back to the no-cycle-counter-available default implementation. Reported-by: Yurij Popov <oss@djphoenix.ru> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- qemu-timer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)