Re: [Qemu-devel] [PATCH for-4.0] compiler.h: Add an explicit check for the compiler version

[Thomas Huth <thuth@redhat.com>]

On 2018-11-30 12:15, Daniel P. Berrangé wrote:
> On Fri, Nov 30, 2018 at 11:43:40AM +0100, Thomas Huth wrote:
>> The questions about our minimum compiler requirement pops up every
>> couple of months, and we then have to recall the details each time.
>> So let's document this in a proper way, by adding a comment and
>> check for the right compiler version to our compiler.h header.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  include/qemu/compiler.h | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
>> index ca9bc85..775446b 100644
>> --- a/include/qemu/compiler.h
>> +++ b/include/qemu/compiler.h
>> @@ -22,6 +22,14 @@
>>  # define QEMU_GNUC_PREREQ(maj, min) 0
>>  #endif
>>  
>> +/*
>> + * We need at least GCC 4.1 for atomics support. Clang also supports these,
>> + * and reports itself as GCC 4.2, so it passes this check, too.
>> + */
>> +#if !QEMU_GNUC_PREREQ(4, 1)
>> +#error QEMU needs a compiler that is compatible with GCC v4.1 or newer
>> +#endif
> 
> This encodes our current minimum which is fine as a first step.
> 
> I think we could reasonably increase our min version now that we
> have declared explicitly what platforms we intend to support

Do we really want to artificially limit our support here without any
further reasons? If the users want to compile QEMU on an older system,
and one of the libraries does not quite match anymore, it's often easy
enough to recompile a newer version of the library to get things going
again. Recompiling a whole compiler is way more cumbersome, though...

OTOH, we could get rid of some more #if QEMU_GNUC_PREREQ spots in the
source code if we bump the minimum version to 4.8 ... so that might be a
real reason to increase the minimum.

> We would need an explicit check for clang, however, instead of
> relying on it claiming gcc 4.2 support - that's a way inaccurate
> claim anyway so detecting a specific clang version would be
> better regardless IMHO

Fine for me if we bump the minimum Clang version to 3.4 ... then we
could get rid of the the CONFIG_INT128 hacks that we have in various
parts of the code.

 Thomas