Message ID | 20201210134752.780923-4-marcandre.lureau@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Remove GCC < 4.8 checks | expand |
On 12/10/20 2:47 PM, marcandre.lureau@redhat.com wrote: > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > Since commit efc6c07 ("configure: Add a test for the minimum compiler > version"), QEMU explicitely depends on GCC >= 4.8. > > (clang >= 3.4 advertizes itself as GCC >= 4.2 compatible and supports > __builtin_expect too) > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > include/qemu/compiler.h | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h > index c76281f354..226ead6c90 100644 > --- a/include/qemu/compiler.h > +++ b/include/qemu/compiler.h > @@ -44,10 +44,6 @@ > #endif > > #ifndef likely > -#if __GNUC__ < 3 > -#define __builtin_expect(x, n) (x) > -#endif > - > #define likely(x) __builtin_expect(!!(x), 1) > #define unlikely(x) __builtin_expect(!!(x), 0) > #endif > Trying with GCC 10: warning: implicit declaration of function ‘likely’ [-Wimplicit-function-declaration] Clang 10: warning: implicit declaration of function 'likely' is invalid in C99 [-Wimplicit-function-declaration] Shouldn't it becleaner to test in the configure script or Meson that likely() and unlikely() are not defined, and define them here unconditionally? Regards, Phil.
On Thu, 10 Dec 2020 at 14:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > On 12/10/20 2:47 PM, marcandre.lureau@redhat.com wrote: > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > > > Since commit efc6c07 ("configure: Add a test for the minimum compiler > > version"), QEMU explicitely depends on GCC >= 4.8. > > > > (clang >= 3.4 advertizes itself as GCC >= 4.2 compatible and supports > > __builtin_expect too) > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Shouldn't it becleaner to test in the configure script or Meson that > likely() and unlikely() are not defined, and define them here > unconditionally? That sounds like way more infrastructure than we need if just checking "is it already defined" is sufficient... -- PMM
Hi On Thu, Dec 10, 2020 at 6:47 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Thu, 10 Dec 2020 at 14:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > > > On 12/10/20 2:47 PM, marcandre.lureau@redhat.com wrote: > > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > > > > > Since commit efc6c07 ("configure: Add a test for the minimum compiler > > > version"), QEMU explicitely depends on GCC >= 4.8. > > > > > > (clang >= 3.4 advertizes itself as GCC >= 4.2 compatible and supports > > > __builtin_expect too) > > > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > > Shouldn't it becleaner to test in the configure script or Meson that > > likely() and unlikely() are not defined, and define them here > > unconditionally? > > That sounds like way more infrastructure than we need if > just checking "is it already defined" is sufficient... > Eh, I am just removing dead-code "#if __GNUC__ < 3". Further cleanups can be done after.
On 10/12/20 15:32, Philippe Mathieu-Daudé wrote: > On 12/10/20 2:47 PM, marcandre.lureau@redhat.com wrote: >> From: Marc-André Lureau <marcandre.lureau@redhat.com> >> >> Since commit efc6c07 ("configure: Add a test for the minimum compiler >> version"), QEMU explicitely depends on GCC >= 4.8. >> >> (clang >= 3.4 advertizes itself as GCC >= 4.2 compatible and supports >> __builtin_expect too) >> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> --- >> include/qemu/compiler.h | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h >> index c76281f354..226ead6c90 100644 >> --- a/include/qemu/compiler.h >> +++ b/include/qemu/compiler.h >> @@ -44,10 +44,6 @@ >> #endif >> >> #ifndef likely >> -#if __GNUC__ < 3 >> -#define __builtin_expect(x, n) (x) >> -#endif >> - >> #define likely(x) __builtin_expect(!!(x), 1) >> #define unlikely(x) __builtin_expect(!!(x), 0) >> #endif >> > > Trying with GCC 10: > warning: implicit declaration of function ‘likely’ > [-Wimplicit-function-declaration] > > Clang 10: > warning: implicit declaration of function 'likely' is invalid in C99 > [-Wimplicit-function-declaration] > > Shouldn't it becleaner to test in the configure script or Meson that > likely() and unlikely() are not defined, and define them here > unconditionally? I think the point of the "#ifndef likely" is that some header file (maybe something from Linux?) might be defining them unexpectedly. So it's difficult to do the test at configure/meson time. I would also tend towards removing the #ifndef and seeing if something breaks, but not as part of this series. Paolo
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h index c76281f354..226ead6c90 100644 --- a/include/qemu/compiler.h +++ b/include/qemu/compiler.h @@ -44,10 +44,6 @@ #endif #ifndef likely -#if __GNUC__ < 3 -#define __builtin_expect(x, n) (x) -#endif - #define likely(x) __builtin_expect(!!(x), 1) #define unlikely(x) __builtin_expect(!!(x), 0) #endif