[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qemu/compiler: Wrap __attribute__((flatten)) in
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH] qemu/compiler: Wrap __attribute__((flatten)) in a macro |
Date: |
Thu, 27 Sep 2018 07:28:44 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 2018-09-26 20:57, Peter Maydell wrote:
> On 26 September 2018 at 16:59, Thomas Huth <address@hidden> wrote:
>
>> +/*
>> + * Clang 3.4 claims to be compatible with GCC 4.2, but does not have the
>> + * "flatten" attribute, so we've got to handle Clang via __has_attribute
>> here
>> + */
>> +#if defined(__clang__) && defined(__has_attribute)
>> +# if __has_attribute(flatten)
>> +# define QEMU_FLATTEN __attribute__((flatten))
>> +# endif
>> +#elif !defined(__clang__) && QEMU_GNUC_PREREQ(4, 1)
>> +# define QEMU_FLATTEN __attribute__((flatten))
>> +#endif
>> +#ifndef QEMU_FLATTEN
>> +# define QEMU_FLATTEN
>> +#endif
>
> I think it would be cleaner to say: if we have __has_attribute,
> trust it; otherwise fall back to version testing. Also, we
> already require at least GCC 4.1 so a version test against
> that is unnecessary. So something like:
>
> #ifndef __has_attribute
> #define __has_attribute(x) 0 / compatibility with older GCC */
> #endif
>
> /*
> * gcc doesn't provide __has_attribute() until gcc 5,
> * but we know all the gcc versions we support have flatten.
> * clang may not have flatten but always has __has_attribute().
> */
> #if __has_attribute(flatten) || !defined(__clang__)
> # define QEMU_FLATTEN __attribute__((flatten))
> #else
> # define QEMU_FLATTEN
> #endif
Good idea, I'll send a v2 with that.
Speaking of the minimum GCC version that we require: Did we ever
officially define that? Or is it just a result from appendix C in our
qemu-doc? I guess the minimum GCC could be defined to 4.2 since that's
the compiler that was still used in OpenBSD recently, before they
switched to Clang?
Maybe we should add a list in our qemu-doc with some minimum versions
that we require (e.g. also glib), so that it is easier to look this up?
Also when searching for GNUC_PREREQ in the sources, there are at least
some occurances in tests/tcg/arm/fcvt.c and
include/fpu/softfloat-macros.h which we could simplify these days...
Thomas