Hi On Fri, Dec 11, 2020 at 5:41 PM Philippe Mathieu-Daudé <phi...@redhat.com> wrote:
> On 12/11/20 2:33 PM, Peter Maydell wrote: > > On Fri, 11 Dec 2020 at 13:13, Philippe Mathieu-Daudé <phi...@redhat.com> > wrote: > >> > >> 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.lur...@redhat.com> > >> [PMD: #error if likely/unlikely already defined] > >> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> > >> --- > >> Supersedes: <20201210134752.780923-4-marcandre.lur...@redhat.com> > >> --- > >> include/qemu/compiler.h | 7 ++----- > >> 1 file changed, 2 insertions(+), 5 deletions(-) > >> > >> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h > >> index c76281f3540..ae1aee79c8d 100644 > >> --- a/include/qemu/compiler.h > >> +++ b/include/qemu/compiler.h > >> @@ -43,14 +43,11 @@ > >> #define tostring(s) #s > >> #endif > >> > >> -#ifndef likely > >> -#if __GNUC__ < 3 > >> -#define __builtin_expect(x, n) (x) > >> +#if defined(likely) || defined(unlikely) > >> +#error building with likely/unlikely is not supported > > > > When exactly will the system headers have 'likely' defined, > > and when would they define it to something other than the > > obvious __builtin_expect() definition the way we do? > > Since there was a check, I tried to be extra-cautious > (better safe than sorry). > > > likely() and unlikely() in my view fall into a category of > > macros like "container_of()" which aren't defined by a system > > header but which do have a standard known set of semantics. > > > > I think there are two reasonable approaches: > > (1) just define the macro, on the assumption that the > > system headers won't have done (because these aren't standard > > macros) > > (2) as we do with container_of() currently, wrap our > > definitions in #ifndef whatever, so that we assume that > > whatever version we might have got from the system is fine > > > > I don't think there's any point in explicitly #error-ing here: > > in fact, it makes the diagnostic to the user less useful, > > because instead of the compiler complaining about the macro > > being defined twice and giving both locations where it was > > defined, now it won't tell the user where the other definition > > was... > > "diagnostic less useful" is a good reason (to invalidate this > patch). > > > I think my preference would be "just drop the ifdef", but > > there isn't much in it really. > > Yes, let's stick to Marc-André v3. > > Thanks for your review! > Ok to r-b v3 then? thanks -- Marc-André Lureau