diff mbox series

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

Message ID 20201210134752.780923-4-marcandre.lureau@redhat.com (mailing list archive)
State New, archived
Headers show
Series Remove GCC < 4.8 checks | expand

Commit Message

Marc-André Lureau Dec. 10, 2020, 1:47 p.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé Dec. 10, 2020, 2:32 p.m. UTC | #1
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.
Peter Maydell Dec. 10, 2020, 2:46 p.m. UTC | #2
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
Marc-André Lureau Dec. 10, 2020, 2:55 p.m. UTC | #3
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.
Paolo Bonzini Dec. 14, 2020, 10:17 a.m. UTC | #4
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 mbox series

Patch

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