All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Markus Armbruster <armbru@redhat.com>,
	QEMU Trivial <qemu-trivial@nongnu.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	"patches@linaro.org" <patches@linaro.org>
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH] Drop QEMU_GNUC_PREREQ() checks for gcc older than 4.1
Date: Tue, 31 Jan 2017 18:11:24 +0000	[thread overview]
Message-ID: <20170131181124.GF20303@redhat.com> (raw)
In-Reply-To: <CAFEAcA-Ggv2VGszZg0Ld3-1wm=hVe8w=4ZcQ7TDRHf5Puq88ug@mail.gmail.com>

On Tue, Jan 31, 2017 at 06:00:13PM +0000, Peter Maydell wrote:
> On 31 January 2017 at 17:40, Markus Armbruster <armbru@redhat.com> wrote:
> > Peter Maydell <peter.maydell@linaro.org> writes:
> >
> >> We already require gcc 4.1 or newer (for the atomic
> >> support), so the fallback codepaths for older gcc
> >> versions than that are now dead code and we can
> >> just delete them.
> >>
> >> NB: clang reports itself as gcc 4.2 (regardless of
> >> clang version), so clang won't be using the fallbacks
> >> either.
> >>
> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >> ---
> >> For compatibility with clang we should probably try to avoid
> >> using QEMU_GNUC_PREREQ() and instead have something in
> >> compiler.h that abstracts away whether the test for "does
> >> the compiler support feature foo" is via a GCC version
> >> check or a clang __has_feature or whatever.
> >
> > Yes, testing for feature is better than testing a version.
> >
> > This patch reduces use of QEMU_GNUC_PREREQ roughly by half.  Good.
> >
> >>
> >>
> >>  include/qemu/compiler.h   |   8 ---
> >>  include/qemu/host-utils.h | 121 ----------------------------------------------
> >>  tcg/arm/tcg-target.h      |   7 ---
> >>  3 files changed, 136 deletions(-)
> >>
> >> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> >> index 157698b..fc12e49 100644
> >> --- a/include/qemu/compiler.h
> >> +++ b/include/qemu/compiler.h
> >> @@ -24,17 +24,9 @@
> >>
> >>  #define QEMU_NORETURN __attribute__ ((__noreturn__))
> >>
> >> -#if QEMU_GNUC_PREREQ(3, 4)
> >>  #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
> >> -#else
> >> -#define QEMU_WARN_UNUSED_RESULT
> >> -#endif
> >
> > Should we inline this macro?
> 
> We have attributes which we wrap in QEMU_ macros already
> even though they always expand to the same thing:
> QEMU_NORETURN and QEMU_ALIGNED. I'm happy to leave these
> to follow that pattern. (If you wanted to send a patch
> series that uninlined all of those then I wouldn't hugely
> object to it, but I think it touches enough files that it's
> a separate thing from removing the #if guards that this
> patch does.)

The other option is just to replace QEMU_WARN_UNUSED_RESULT with

  #define QEMU_WARN_UNUSED_RESULT  G_GNUC_WARN_UNUSED_RESULT

and convert code to use G_GNUC_WARN_UNUSED_RESULT directly until we
can kill the QEMU specific define. There's no benefit to QEMU having
its own defines that duplicate stuff already covered by our min
required glib - G_GNUC_WARN_UNUSED_RESULT was added in 2.10 for
example.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|


WARNING: multiple messages have this Message-ID (diff)
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Markus Armbruster <armbru@redhat.com>,
	QEMU Trivial <qemu-trivial@nongnu.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	"patches@linaro.org" <patches@linaro.org>
Subject: Re: [Qemu-devel] [PATCH] Drop QEMU_GNUC_PREREQ() checks for gcc older than 4.1
Date: Tue, 31 Jan 2017 18:11:24 +0000	[thread overview]
Message-ID: <20170131181124.GF20303@redhat.com> (raw)
In-Reply-To: <CAFEAcA-Ggv2VGszZg0Ld3-1wm=hVe8w=4ZcQ7TDRHf5Puq88ug@mail.gmail.com>

On Tue, Jan 31, 2017 at 06:00:13PM +0000, Peter Maydell wrote:
> On 31 January 2017 at 17:40, Markus Armbruster <armbru@redhat.com> wrote:
> > Peter Maydell <peter.maydell@linaro.org> writes:
> >
> >> We already require gcc 4.1 or newer (for the atomic
> >> support), so the fallback codepaths for older gcc
> >> versions than that are now dead code and we can
> >> just delete them.
> >>
> >> NB: clang reports itself as gcc 4.2 (regardless of
> >> clang version), so clang won't be using the fallbacks
> >> either.
> >>
> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >> ---
> >> For compatibility with clang we should probably try to avoid
> >> using QEMU_GNUC_PREREQ() and instead have something in
> >> compiler.h that abstracts away whether the test for "does
> >> the compiler support feature foo" is via a GCC version
> >> check or a clang __has_feature or whatever.
> >
> > Yes, testing for feature is better than testing a version.
> >
> > This patch reduces use of QEMU_GNUC_PREREQ roughly by half.  Good.
> >
> >>
> >>
> >>  include/qemu/compiler.h   |   8 ---
> >>  include/qemu/host-utils.h | 121 ----------------------------------------------
> >>  tcg/arm/tcg-target.h      |   7 ---
> >>  3 files changed, 136 deletions(-)
> >>
> >> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> >> index 157698b..fc12e49 100644
> >> --- a/include/qemu/compiler.h
> >> +++ b/include/qemu/compiler.h
> >> @@ -24,17 +24,9 @@
> >>
> >>  #define QEMU_NORETURN __attribute__ ((__noreturn__))
> >>
> >> -#if QEMU_GNUC_PREREQ(3, 4)
> >>  #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
> >> -#else
> >> -#define QEMU_WARN_UNUSED_RESULT
> >> -#endif
> >
> > Should we inline this macro?
> 
> We have attributes which we wrap in QEMU_ macros already
> even though they always expand to the same thing:
> QEMU_NORETURN and QEMU_ALIGNED. I'm happy to leave these
> to follow that pattern. (If you wanted to send a patch
> series that uninlined all of those then I wouldn't hugely
> object to it, but I think it touches enough files that it's
> a separate thing from removing the #if guards that this
> patch does.)

The other option is just to replace QEMU_WARN_UNUSED_RESULT with

  #define QEMU_WARN_UNUSED_RESULT  G_GNUC_WARN_UNUSED_RESULT

and convert code to use G_GNUC_WARN_UNUSED_RESULT directly until we
can kill the QEMU specific define. There's no benefit to QEMU having
its own defines that duplicate stuff already covered by our min
required glib - G_GNUC_WARN_UNUSED_RESULT was added in 2.10 for
example.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

  reply	other threads:[~2017-01-31 18:11 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-31 16:14 [Qemu-trivial] [PATCH] Drop QEMU_GNUC_PREREQ() checks for gcc older than 4.1 Peter Maydell
2017-01-31 16:14 ` [Qemu-devel] " Peter Maydell
2017-01-31 16:55 ` [Qemu-trivial] " Daniel P. Berrange
2017-01-31 16:55   ` Daniel P. Berrange
2017-01-31 16:58   ` [Qemu-trivial] " Peter Maydell
2017-01-31 16:58     ` Peter Maydell
2017-01-31 23:27     ` [Qemu-trivial] " Paolo Bonzini
2017-01-31 23:27       ` Paolo Bonzini
2017-01-31 17:40 ` [Qemu-trivial] " Markus Armbruster
2017-01-31 17:40   ` Markus Armbruster
2017-01-31 18:00   ` [Qemu-trivial] " Peter Maydell
2017-01-31 18:00     ` Peter Maydell
2017-01-31 18:11     ` Daniel P. Berrange [this message]
2017-01-31 18:11       ` Daniel P. Berrange
2017-01-31 18:32       ` [Qemu-trivial] " Peter Maydell
2017-01-31 18:32         ` Peter Maydell
2017-01-31 19:02         ` [Qemu-trivial] " Markus Armbruster
2017-01-31 19:02           ` Markus Armbruster
2017-01-31 18:58     ` [Qemu-trivial] " Markus Armbruster
2017-01-31 18:58       ` Markus Armbruster
2017-01-31 23:27   ` [Qemu-trivial] " Paolo Bonzini
2017-01-31 23:27     ` Paolo Bonzini
2017-02-01  6:49     ` [Qemu-trivial] " Markus Armbruster
2017-02-01  6:49       ` Markus Armbruster
2017-02-02  1:09       ` [Qemu-trivial] " Paolo Bonzini
2017-02-02  1:09         ` Paolo Bonzini
2017-02-02 10:17         ` [Qemu-trivial] " Markus Armbruster
2017-02-02 10:17           ` Markus Armbruster
2017-04-21  9:08 ` [Qemu-trivial] " Peter Maydell
2017-04-21  9:08   ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170131181124.GF20303@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=patches@linaro.org \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.