qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 02/13] qemu/atomic: Drop special case for unsupported comp


From: Marc-André Lureau
Subject: Re: [PATCH v2 02/13] qemu/atomic: Drop special case for unsupported compiler
Date: Thu, 10 Dec 2020 17:17:05 +0400

Hi

On Thu, Nov 26, 2020 at 4:23 PM Peter Maydell <peter.maydell@linaro.org> wrote:
On Thu, 26 Nov 2020 at 12:06, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Nov 26, 2020 at 11:49:28AM +0000, Peter Maydell wrote:
> > On Thu, 26 Nov 2020 at 11:29, <marcandre.lureau@redhat.com> wrote:
> > >
> > > From: Philippe Mathieu-Daudé <philmd@redhat.com>
> > >
> > > Since commit efc6c070aca ("configure: Add a test for the
> > > minimum compiler version") the minimum compiler version
> > > required for GCC is 4.8, which has the GCC BZ#36793 bug fixed.
> > >
> > > We can safely remove the special case introduced in commit
> > > a281ebc11a6 ("virtio: add missing mb() on notification").
> > >
> > > With clang 3.8 (xenial amd64) __ATOMIC_RELAXED is defined, so the chunk
> > > to remove (which is x86-specific), isn't reached.
> >
> > The minimum clang version enforced by configure is 3.4, not 3.8.
> > (Or Apple XCode clang 5.1 -- they use a different versioning scheme!)
>
> We picked clang 3.4 based on fact that is what ships in EPEL7, and
> Debian Jessie 3.5.  We then picked the XCode version to match.
>
> Based on our platform support matrix we no longer support Debian
> Jessie, and IMHO we also don't really need to consider 3rd party
> add-on repos shipping non-default toolschains. So IMHO we could
> entirely ignore clang in EPEL7 when picking min versions.
>
> IOW, we are likely justified in picking a new clang version if
> someone wants to research what is a suitable min version across
> our intended supported distros.

Sure, but if we do that then the series should start with the
"bump the minimum clang version" patch with accompanying
justification.


With clang-3.4.2-9.el7.x86_64 (epel7), __ATOMIC_RELAXED is defined. I'll update the commit message.

Some research on google suggests that it might be true also with XCode clang 5.1, I could use some help to verify that:
clang -dM -E - < /dev/null | grep __ATOMIC_RELAXED


--
Marc-André Lureau

reply via email to

[Prev in Thread] Current Thread [Next in Thread]