Re: [Qemu-devel] [PATCH] qemu/compiler: Wrap __attribute__((flatten)) in a macro

[Thomas Huth]

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