compiler.h: Add an explicit check for the compiler version

PATCH Applied Thomas Huth 3 participants 8 messages Nov 30, 2018

Share & tools

Thomas Huth

author

2018-11-30 10:43 UTC #0 lore raw

PATCH 1/1 compiler.h: Add an explicit check for the compiler version

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 <[email protected]>

---
 include/qemu/compiler.h | 8 ++++++++
 1 file changed, 8 insertions(+)

include/qemu/compiler.h +8 -0

index ca9bc85..775446b 100644
@@ -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
+
 #define QEMU_NORETURN __attribute__ ((__noreturn__))
 
 #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))

Signature · 1 line

1.8.3.1

Paolo Bonzini

Applied

2018-11-30 10:46 UTC #1 lore raw

Quoted 20 lines · Thomas Huth

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 <[email protected]>
---
 include/qemu/compiler.h | 8 ++++++++
 1 file changed, 8 insertions(+)
include/qemu/compiler.h +1 -0
index ca9bc85..775446b 100644
@@ -22,6 +22,14 @@
 # define QEMU_GNUC_PREREQ(maj, min) 0
 #endif
 
+/*

include/qemu/compiler.h line 35

Quoted 10 lines

include/qemu/compiler.h +7 -0

+ * 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
+
 #define QEMU_NORETURN __attribute__ ((__noreturn__))
 
 #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))

Queued, thanks.

Paolo

Daniel P. Berrangé

2018-11-30 11:16 UTC #2 lore raw

In reply to Thomas Huth

Quoted 15 lines · Thomas Huth

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 <[email protected]>
---
 include/qemu/compiler.h | 8 ++++++++
 1 file changed, 8 insertions(+)
include/qemu/compiler.h +0 -0
index ca9bc85..775446b 100644

include/qemu/compiler.h line 31

Quoted 10 lines

include/qemu/compiler.h +7 -0

@@ -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

      RHEL-7: 4.8.5
      Debian (Stretch): 6.3.0
      Debian (Jessie): 4.8.4
      OpenBSD (ports): 4.9.4
      FreeBSD (ports): 8.2.0
      OpenSUSE Leap 15: 7.3.1
      Ubuntu (Xenial): 5.3.1
      macOS (Homebrew): 8.2.0

Arguably we don't care about gcc version for *BSD and macOS since those platforms normally use CLang

Anyway, with this info I think we can reasonably pick gcc 4.8.0

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

Regards,
Daniel

Signature · 3 lines

|: 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 :|

Thomas Huth

author

2018-11-30 12:30 UTC #3 lore raw

Quoted 29 lines · Daniel P. Berrangé

Quoted 15 lines · Thomas Huth
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 <[email protected]>
---
 include/qemu/compiler.h | 8 ++++++++
 1 file changed, 8 insertions(+)
include/qemu/compiler.h +0 -0
index ca9bc85..775446b 100644
include/qemu/compiler.h line 31
Quoted 10 lines
include/qemu/compiler.h +7 -0
@@ -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.

Quoted 2 lines

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.

Quoted 4 lines

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

Paolo Bonzini

2018-11-30 12:50 UTC #4 lore raw

Quoted 9 lines · Thomas Huth

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.

Of all of them, I think, if we bump to 4.7?

Older 4.6 and 4.7 version have the PR55489 bug that we're checking for in configure, too ("we should be able to delete this at the end of 2013" :)). Bumping to 4.8 would let us remove TRANSLATE_OPT_CFLAGS for good.

Paolo

Quoted 11 lines

Quoted 4 lines
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

Thomas Huth

author

2018-11-30 12:52 UTC #5 lore raw

Quoted 11 lines · Paolo Bonzini

Quoted 9 lines · Thomas Huth
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.

Quoted 5 lines

Of all of them, I think, if we bump to 4.7?
Older 4.6 and 4.7 version have the PR55489 bug that we're checking for in configure, too ("we should be able to delete this at the end of 2013" :)). Bumping to 4.8 would let us remove TRANSLATE_OPT_CFLAGS for good.

Ok, sounds like bumping to 4.8 would really help to get rid of some legacy stuff ... so please drop this patch for compiler.h, I'll try to come up with some other patches for checking the GCC version and Clang version in the configure script already instead.

Thomas

Daniel P. Berrangé

2018-11-30 14:47 UTC #6 lore raw

Quoted 14 lines · Thomas Huth

Quoted 13 lines · Paolo Bonzini
Quoted 9 lines · Thomas Huth
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.
Of all of them, I think, if we bump to 4.7?

Quoted 8 lines

Quoted 3 lines
Older 4.6 and 4.7 version have the PR55489 bug that we're checking for in configure, too ("we should be able to delete this at the end of 2013" :)). Bumping to 4.8 would let us remove TRANSLATE_OPT_CFLAGS for good.
Ok, sounds like bumping to 4.8 would really help to get rid of some legacy stuff ... so please drop this patch for compiler.h, I'll try to come up with some other patches for checking the GCC version and Clang version in the configure script already instead.

BTW, when checking clang version there's some strangeness with macOS which reports completely different versions from the clang it was derived from, so we'd require separate checks for normal clang vs apple's clang.

Regards,
Daniel

Signature · 3 lines

|: 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 :|

Daniel P. Berrangé

2018-11-30 13:04 UTC #7 lore raw

In reply to Thomas Huth

Quoted 30 lines · Thomas Huth

Quoted 29 lines · Daniel P. Berrangé
Quoted 15 lines · Thomas Huth
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 <[email protected]>
---
 include/qemu/compiler.h | 8 ++++++++
 1 file changed, 8 insertions(+)
include/qemu/compiler.h +0 -0
index ca9bc85..775446b 100644
include/qemu/compiler.h line 31
Quoted 10 lines
include/qemu/compiler.h +7 -0
@@ -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.

Quoted 8 lines

Quoted 2 lines
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...

The whole point of declaring our supported platforms is that we'll explicitly not care about whether we can build on older platforms, regardless of whether the incompatibility is with libraries or toolchains or something else. It gives us a clear rule on when we're able to drop back compat support in the code for older tools chains or libraries, reducing our own maint burden long term.

We fully expect some users will be inconvenienced by this, but the benefits to our maint workload will outweigh this cost. This lets us deliver a better project to the vast majority of our userbase who are not on ancient platforms.

Quoted 3 lines

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.

Yes, exactly

Quoted 8 lines

Quoted 4 lines
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.

Indeed that would be good too.

Regards,
Daniel

Signature · 3 lines

|: 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 :|