Re: [PATCH v3 03/13] compiler.h: remove GCC < 3 __builtin_expect fallback

[Paolo Bonzini <pbonzini@xxxxxxxxxx>]

On 10/12/20 15:32, Philippe Mathieu-Daudé wrote:
> On 12/10/20 2:47 PM, marcandre.lureau@xxxxxxxxxx wrote:
> > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
> >   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