From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=BAYES_00,DATE_IN_PAST_03_06, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1B097C4361B for ; Sat, 12 Dec 2020 19:12:08 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7847C22257 for ; Sat, 12 Dec 2020 19:12:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7847C22257 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:40762 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1koAJK-0002hM-IO for qemu-devel@archiver.kernel.org; Sat, 12 Dec 2020 14:12:06 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:46306) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ko9zg-0005t7-TJ for qemu-devel@nongnu.org; Sat, 12 Dec 2020 13:51:50 -0500 Received: from mail-lf1-x143.google.com ([2a00:1450:4864:20::143]:38765) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1ko9zX-0005zo-3M for qemu-devel@nongnu.org; Sat, 12 Dec 2020 13:51:47 -0500 Received: by mail-lf1-x143.google.com with SMTP id w13so19943717lfd.5 for ; Sat, 12 Dec 2020 10:51:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=QyqyyItwuoGulxpcnRRggwj6hwYpsDqivibechKudEA=; b=NY2jCtCPBqW61qJmTeDozGQMkiOEfS1InkXcWPA7UupHsKywPP95fG2VLVxfRns3fH oJ2529UDKMPx9jx4MMAOzxQI/NIiFGJunjCLn5UvvwdM5N2ZXrZ5a9nEnUvVZBbYsHVA Tnav6GClZybj13HsCgVFHfsLlwdJWNg2zbeVTKFCZlw8BW5C4zNEdyvJtiHmSPTWJhcO idMvs1NRqGCXGiFitKNhMFb9rnVxnr2KBeCd45Gx/i4h/zz/zDlswJI07fK61NTQ5648 smI1q/IB0yDlOvxtkOk5RZbnnOLj1I8Sk6xiuH0zv8WjDP2DPvcE+VxShAtq6BmXUyPS t+5Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=QyqyyItwuoGulxpcnRRggwj6hwYpsDqivibechKudEA=; b=hw3mn/2rN3ZV3fzx64oT6dGJA6QxkpZn5VzyKl9YfIOqumnA+ipx9ZK84oBIJH/F5m 7uYslbEXcJ/vUo5xAI8FI2raJKdkgRh1lWxsztQltmo9ITN51luh4qJnRj5E36Jpmg2z wfsLzTqVVDzG/LGgC0TVWybzbto/OBWdfPgAzeFAHEaS/CzX7ImU/SB9TzuPbvm3/gKz A7dAW8fouvn0s5vwui7TGUAXy610k6VBLpk8pqb5XU7UsSJaqsPBFZhk0iu4uVMjKgk+ aE048XZ6ekkURBHDUMmmOvOAq5xyEenfnJGt1ut3OgqT/KEXHuGCF+jBjCi15UkE2MK+ bZRA== X-Gm-Message-State: AOAM531OEYN58ardgYAIVlZia6Bb5Z9oGnxs1uZZ8qj40IiH/vL9Lho6 aRXO85QF7E7DuuaaJ3gOd7AjPw2nnknoqahQBCCPvhUaook= X-Google-Smtp-Source: ABdhPJyVbZ0AZjx6E7Uw+WQ9G9CbHlJjUaXSjslMCZhXwVcREkjp1D4bMYXW96YCw1p0amnjT1LFcy1UtIUsbpLIRYc= X-Received: by 2002:a50:a6de:: with SMTP id f30mr17051378edc.30.1607781100511; Sat, 12 Dec 2020 05:51:40 -0800 (PST) MIME-Version: 1.0 References: <20201211131346.473882-1-philmd@redhat.com> <86831153-46c0-3bea-dd24-594ef258c1f4@redhat.com> In-Reply-To: <86831153-46c0-3bea-dd24-594ef258c1f4@redhat.com> From: =?UTF-8?B?TWFyYy1BbmRyw6kgTHVyZWF1?= Date: Sat, 12 Dec 2020 17:51:28 +0400 Message-ID: Subject: Re: [RFC PATCH v4] compiler.h: remove GCC < 3 __builtin_expect fallback To: =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= Content-Type: multipart/alternative; boundary="0000000000005f658a05b644b5ba" Received-SPF: pass client-ip=2a00:1450:4864:20::143; envelope-from=marcandre.lureau@gmail.com; helo=mail-lf1-x143.google.com X-Spam_score_int: -4 X-Spam_score: -0.5 X-Spam_bar: / X-Spam_report: (-0.5 / 5.0 requ) BAYES_00=-1.9, DATE_IN_PAST_03_06=1.592, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001, T_SPF_HELO_TEMPERROR=0.01 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Maydell , QEMU Developers Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --0000000000005f658a05b644b5ba Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi On Fri, Dec 11, 2020 at 5:41 PM Philippe Mathieu-Daud=C3=A9 wrote: > On 12/11/20 2:33 PM, Peter Maydell wrote: > > On Fri, 11 Dec 2020 at 13:13, Philippe Mathieu-Daud=C3=A9 > wrote: > >> > >> Since commit efc6c07 ("configure: Add a test for the minimum compiler > >> version"), QEMU explicitely depends on GCC >=3D 4.8. > >> > >> (clang >=3D 3.4 advertizes itself as GCC >=3D 4.2 compatible and suppo= rts > >> __builtin_expect too) > >> > >> Signed-off-by: Marc-Andr=C3=A9 Lureau > >> [PMD: #error if likely/unlikely already defined] > >> Signed-off-by: Philippe Mathieu-Daud=C3=A9 > >> --- > >> Supersedes: <20201210134752.780923-4-marcandre.lureau@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=C3=A9 v3. > > Thanks for your review! > Ok to r-b v3 then? thanks --=20 Marc-Andr=C3=A9 Lureau --0000000000005f658a05b644b5ba Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi

On Fri, Dec 11, 2020 at 5:41 PM Phi= lippe Mathieu-Daud=C3=A9 <philmd@re= dhat.com> wrote:
On 12/11/20 2:33 PM, Peter Maydell wrote:
> On Fri, 11 Dec 2020 at 13:13, Philippe Mathieu-Daud=C3=A9 <philmd@redhat.com> w= rote:
>>
>> Since commit efc6c07 ("configure: Add a test for the minimum = compiler
>> version"), QEMU explicitely depends on GCC >=3D 4.8.
>>
>> (clang >=3D 3.4 advertizes itself as GCC >=3D 4.2 compatible= and supports
>> __builtin_expect too)
>>
>> Signed-off-by: Marc-Andr=C3=A9 Lureau <marcandre.lureau@redhat.com>= ;
>> [PMD: #error if likely/unlikely already defined]
>> Signed-off-by: Philippe Mathieu-Daud=C3=A9 <philmd@redhat.com>
>> ---
>> Supersedes: <20201210134752.780923-4-marcandre.lu= reau@redhat.com>
>> ---
>>=C2=A0 include/qemu/compiler.h | 7 ++-----
>>=C2=A0 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 @@
>>=C2=A0 #define tostring(s)=C2=A0 =C2=A0 #s
>>=C2=A0 #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 s= ystem
> header but which do have a standard known set of semantics.
>
> I think there are two reasonable approaches:
>=C2=A0 (1) just define the macro, on the assumption that the
> system headers won't have done (because these aren't standard<= br> > macros)
>=C2=A0 (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=C3=A9 v3.

Thanks for your review!

Ok to r-b v3 th= en?
thanks



--
Marc-Andr=C3=A9 Lureau
--0000000000005f658a05b644b5ba--