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

[Daniel P . Berrangé]

On Thu, Dec 10, 2020 at 05:17:05PM +0400, Marc-André Lureau wrote:
> 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

The oldest xcode that travis has support for is 7.3. I'd really just
suggest bumping min clang to something more modern, than trying to
find xcode 5.1
 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|