[PATCH v2 00/13] Remove GCC < 4.8 checks

[PATCH v2 00/13] Remove GCC < 4.8 checks

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Hi,

Since commit efc6c07 ("configure: Add a test for the minimum compiler version"),
QEMU explicitely depends on GCC >= 4.8.

v2:
 - include reviewed Philippe earlier series
 - drop problematic patch to replace GCC_FMT_ATTR, but tweak the check to be clang
 - replace QEMU_GNUC_PREREQ with G_GNUC_CHECK_VERSION
 - split changes
 - add patches to drop __GNUC__ checks (clang advertizes itself as 4.2.1, unless
   -fgnuc-version=0)

Marc-André Lureau (11):
  compiler.h: replace QEMU_GNUC_PREREQ macro
  compiler.h: remove GCC < 3 __builtin_expect fallback
  qemu-plugin.h: remove GCC < 4
  tests: remove GCC < 4 fallbacks
  virtiofsd: replace _Static_assert with QEMU_BUILD_BUG_ON
  compiler.h: explicit case for Clang printf attribute
  audio: remove GNUC & MSVC check
  poison: remove GNUC check
  xen: remove GNUC check
  compiler: remove GNUC check
  linux-user: remove GNUC check

Philippe Mathieu-Daudé (2):
  qemu/atomic: Drop special case for unsupported compiler
  accel/tcg: Remove special case for GCC < 4.6

 include/exec/poison.h              |  2 --
 include/hw/xen/interface/io/ring.h |  9 ------
 include/qemu/atomic.h              | 17 -----------
 include/qemu/compiler.h            | 45 ++++++++----------------------
 include/qemu/qemu-plugin.h         |  9 ++----
 scripts/cocci-macro-file.h         |  1 -
 tools/virtiofsd/fuse_common.h      | 11 +-------
 accel/tcg/cpu-exec.c               |  2 +-
 audio/audio.c                      |  8 +-----
 linux-user/strace.c                |  4 ---
 tests/tcg/arm/fcvt.c               |  8 ++----
 11 files changed, 20 insertions(+), 96 deletions(-)

-- 
2.29.0

[PATCH v2 01/13] compiler.h: replace QEMU_GNUC_PREREQ macro

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Replace it with glib G_GNUC_CHECK_VERSION.

Available since 2.42, the macro received a small correction in
2.55.1 (glib d44afbadda8a "macros: make G_GNUC_CHECK_VERSION()
portable" which was apparently harmless).

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/atomic.h      |  2 +-
 include/qemu/compiler.h    | 15 +++------------
 scripts/cocci-macro-file.h |  1 -
 accel/tcg/cpu-exec.c       |  2 +-
 4 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index c1d211a351..c409257c11 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -248,7 +248,7 @@
  * 4.3 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36793).
  */
 #if defined(__i386__) || defined(__x86_64__)
-#if !QEMU_GNUC_PREREQ(4, 4)
+#if !G_GNUC_CHECK_VERSION(4, 4)
 #if defined __x86_64__
 #define smp_mb()    ({ asm volatile("mfence" ::: "memory"); (void)0; })
 #else
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index c76281f354..cf044bab4a 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -7,21 +7,12 @@
 #ifndef COMPILER_H
 #define COMPILER_H
 
+#include "glib-compat.h"
+
 #if defined __clang_analyzer__ || defined __COVERITY__
 #define QEMU_STATIC_ANALYSIS 1
 #endif
 
-/*----------------------------------------------------------------------------
-| The macro QEMU_GNUC_PREREQ tests for minimum version of the GNU C compiler.
-| The code is a copy of SOFTFLOAT_GNUC_PREREQ, see softfloat-macros.h.
-*----------------------------------------------------------------------------*/
-#if defined(__GNUC__) && defined(__GNUC_MINOR__)
-# define QEMU_GNUC_PREREQ(maj, min) \
-         ((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min))
-#else
-# define QEMU_GNUC_PREREQ(maj, min) 0
-#endif
-
 #define QEMU_NORETURN __attribute__ ((__noreturn__))
 
 #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
@@ -104,7 +95,7 @@
                                    sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)))
 
 #if defined __GNUC__
-# if !QEMU_GNUC_PREREQ(4, 4)
+# if !G_GNUC_CHECK_VERSION(4, 4)
    /* gcc versions before 4.4.x don't support gnu_printf, so use printf. */
 #  define GCC_FMT_ATTR(n, m) __attribute__((format(printf, n, m)))
 # else
diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h
index c6bbc05ba3..20eea6b708 100644
--- a/scripts/cocci-macro-file.h
+++ b/scripts/cocci-macro-file.h
@@ -19,7 +19,6 @@
  */
 
 /* From qemu/compiler.h */
-#define QEMU_GNUC_PREREQ(maj, min) 1
 #define QEMU_NORETURN __attribute__ ((__noreturn__))
 #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
 #define QEMU_SENTINEL __attribute__((sentinel))
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 58aea605d8..21a46d2e85 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -724,7 +724,7 @@ int cpu_exec(CPUState *cpu)
 
     /* prepare setjmp context for exception handling */
     if (sigsetjmp(cpu->jmp_env, 0) != 0) {
-#if defined(__clang__) || !QEMU_GNUC_PREREQ(4, 6)
+#if defined(__clang__) || !G_GNUC_CHECK_VERSION(4, 6)
         /* Some compilers wrongly smash all local variables after
          * siglongjmp. There were bug reports for gcc 4.5.0 and clang.
          * Reload essential local variables here for those compilers.
-- 
2.29.0

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

[marcandre.lureau]

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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/atomic.h | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index c409257c11..8f4b3a80fb 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -241,23 +241,6 @@
 
 #else /* __ATOMIC_RELAXED */
 
-/*
- * We use GCC builtin if it's available, as that can use mfence on
- * 32-bit as well, e.g. if built with -march=pentium-m. However, on
- * i386 the spec is buggy, and the implementation followed it until
- * 4.3 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36793).
- */
-#if defined(__i386__) || defined(__x86_64__)
-#if !G_GNUC_CHECK_VERSION(4, 4)
-#if defined __x86_64__
-#define smp_mb()    ({ asm volatile("mfence" ::: "memory"); (void)0; })
-#else
-#define smp_mb()    ({ asm volatile("lock; addl $0,0(%%esp) " ::: "memory"); (void)0; })
-#endif
-#endif
-#endif
-
-
 #ifdef __alpha__
 #define smp_read_barrier_depends()   asm volatile("mb":::"memory")
 #endif
-- 
2.29.0

[PATCH v2 03/13] accel/tcg: Remove special case for GCC < 4.6

[marcandre.lureau]

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.

We can safely remove the special case for GCC 4.6 introduced
in commit 0448f5f8b81 ("cpu-exec: Fix compiler warning
(-Werror=clobbered)").
No change for Clang as we don't know.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 accel/tcg/cpu-exec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 21a46d2e85..37a88edb6d 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -724,7 +724,7 @@ int cpu_exec(CPUState *cpu)
 
     /* prepare setjmp context for exception handling */
     if (sigsetjmp(cpu->jmp_env, 0) != 0) {
-#if defined(__clang__) || !G_GNUC_CHECK_VERSION(4, 6)
+#if defined(__clang__)
         /* Some compilers wrongly smash all local variables after
          * siglongjmp. There were bug reports for gcc 4.5.0 and clang.
          * Reload essential local variables here for those compilers.
-- 
2.29.0

[PATCH v2 04/13] compiler.h: remove GCC < 3 __builtin_expect fallback

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Since commit efc6c07 ("configure: Add a test for the minimum compiler
version"), QEMU explicitely depends on GCC >= 4.8.

(clang >= 3.8 advertizes itself as GCC >= 4.2 compatible)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/compiler.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index cf044bab4a..ae3e0df34c 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -35,10 +35,6 @@
 #endif
 
 #ifndef likely
-#if __GNUC__ < 3
-#define __builtin_expect(x, n) (x)
-#endif
-
 #define likely(x)   __builtin_expect(!!(x), 1)
 #define unlikely(x)   __builtin_expect(!!(x), 0)
 #endif
-- 
2.29.0

[PATCH v2 05/13] qemu-plugin.h: remove GCC < 4

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Since commit efc6c07 ("configure: Add a test for the minimum compiler
version"), QEMU explicitely depends on GCC >= 4.8.

(clang >= 3.8 advertizes itself as GCC >= 4.2 compatible)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/qemu-plugin.h | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index bab8b0d4b3..5775e82c4e 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -28,13 +28,8 @@
   #endif
   #define QEMU_PLUGIN_LOCAL
 #else
-  #if __GNUC__ >= 4
-    #define QEMU_PLUGIN_EXPORT __attribute__((visibility("default")))
-    #define QEMU_PLUGIN_LOCAL  __attribute__((visibility("hidden")))
-  #else
-    #define QEMU_PLUGIN_EXPORT
-    #define QEMU_PLUGIN_LOCAL
-  #endif
+  #define QEMU_PLUGIN_EXPORT __attribute__((visibility("default")))
+  #define QEMU_PLUGIN_LOCAL  __attribute__((visibility("hidden")))
 #endif
 
 typedef uint64_t qemu_plugin_id_t;
-- 
2.29.0

[PATCH v2 06/13] tests: remove GCC < 4 fallbacks

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Since commit efc6c07 ("configure: Add a test for the minimum compiler
version"), QEMU explicitely depends on GCC >= 4.8.

(clang >= 3.8 advertizes itself as GCC >= 4.2 compatible)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/tcg/arm/fcvt.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/tests/tcg/arm/fcvt.c b/tests/tcg/arm/fcvt.c
index 617626bc63..7ac47b564e 100644
--- a/tests/tcg/arm/fcvt.c
+++ b/tests/tcg/arm/fcvt.c
@@ -73,11 +73,9 @@ static void print_int64(int i, int64_t num)
 
 #ifndef SNANF
 /* Signaling NaN macros, if supported.  */
-# if __GNUC_PREREQ(3, 3)
-#  define SNANF (__builtin_nansf (""))
-#  define SNAN (__builtin_nans (""))
-#  define SNANL (__builtin_nansl (""))
-# endif
+# define SNANF (__builtin_nansf (""))
+# define SNAN (__builtin_nans (""))
+# define SNANL (__builtin_nansl (""))
 #endif
 
 float single_numbers[] = { -SNANF,
-- 
2.29.0

[PATCH v2 07/13] virtiofsd: replace _Static_assert with QEMU_BUILD_BUG_ON

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

This allows to get rid of a check for older GCC version (which was a bit
bogus too since it was falling back on c++ version..)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tools/virtiofsd/fuse_common.h | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/tools/virtiofsd/fuse_common.h b/tools/virtiofsd/fuse_common.h
index 5aee5193eb..a2484060b6 100644
--- a/tools/virtiofsd/fuse_common.h
+++ b/tools/virtiofsd/fuse_common.h
@@ -809,15 +809,6 @@ void fuse_remove_signal_handlers(struct fuse_session *se);
  *
  * On 32bit systems please add -D_FILE_OFFSET_BITS=64 to your compile flags!
  */
-
-#if defined(__GNUC__) &&                                      \
-    (__GNUC__ > 4 || __GNUC__ == 4 && __GNUC_MINOR__ >= 6) && \
-    !defined __cplusplus
-_Static_assert(sizeof(off_t) == 8, "fuse: off_t must be 64bit");
-#else
-struct _fuse_off_t_must_be_64bit_dummy_struct {
-    unsigned _fuse_off_t_must_be_64bit:((sizeof(off_t) == 8) ? 1 : -1);
-};
-#endif
+QEMU_BUILD_BUG_ON(sizeof(off_t) != 8);
 
 #endif /* FUSE_COMMON_H_ */
-- 
2.29.0

[PATCH v2 08/13] compiler.h: explicit case for Clang printf attribute

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Since commit efc6c07 ("configure: Add a test for the minimum compiler
version"), QEMU explicitely depends on GCC >= 4.8, we could thus drop
earlier version checks. Except clang advertizes itself as GCC 4.2.1.

Since clang doesn't support gnu_printf, make that case explicitely and
drop GCC version check.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/compiler.h | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index ae3e0df34c..108bfdb391 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -90,18 +90,16 @@
 #define QEMU_BUILD_BUG_ON_ZERO(x) (sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)) - \
                                    sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)))
 
-#if defined __GNUC__
-# if !G_GNUC_CHECK_VERSION(4, 4)
-   /* gcc versions before 4.4.x don't support gnu_printf, so use printf. */
-#  define GCC_FMT_ATTR(n, m) __attribute__((format(printf, n, m)))
-# else
-   /* Use gnu_printf when supported (qemu uses standard format strings). */
-#  define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
-#  if defined(_WIN32)
-    /* Map __printf__ to __gnu_printf__ because we want standard format strings
-     * even when MinGW or GLib include files use __printf__. */
-#   define __printf__ __gnu_printf__
-#  endif
+#if defined(__clang__)
+  /* clang doesn't support gnu_printf, so use printf. */
+# define GCC_FMT_ATTR(n, m) __attribute__((format(printf, n, m)))
+#elif defined(__GNUC__)
+   /* Use gnu_printf (qemu uses standard format strings). */
+# define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
+# if defined(_WIN32)
+   /* Map __printf__ to __gnu_printf__ because we want standard format strings
+    * even when MinGW or GLib include files use __printf__. */
+#  define __printf__ __gnu_printf__
 # endif
 #else
 #define GCC_FMT_ATTR(n, m)
-- 
2.29.0

[PATCH v2 09/13] audio: remove GNUC & MSVC check

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

QEMU requires either GCC or Clang, which both advertize __GNUC__.
Drop MSVC fallback path.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 audio/audio.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 46578e4a58..d7a00294de 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -122,13 +122,7 @@ int audio_bug (const char *funcname, int cond)
 
 #if defined AUDIO_BREAKPOINT_ON_BUG
 #  if defined HOST_I386
-#    if defined __GNUC__
-        __asm__ ("int3");
-#    elif defined _MSC_VER
-        _asm _emit 0xcc;
-#    else
-        abort ();
-#    endif
+      __asm__ ("int3");
 #  else
         abort ();
 #  endif
-- 
2.29.0

[PATCH v2 10/13] poison: remove GNUC check

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

QEMU requires Clang or GCC, that define and support __GNUC__ extensions

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/exec/poison.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/exec/poison.h b/include/exec/poison.h
index 7b9ac361dc..d7ae1f23e7 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -3,7 +3,6 @@
 
 #ifndef HW_POISON_H
 #define HW_POISON_H
-#ifdef __GNUC__
 
 #pragma GCC poison TARGET_I386
 #pragma GCC poison TARGET_X86_64
@@ -93,4 +92,3 @@
 #pragma GCC poison CONFIG_SOFTMMU
 
 #endif
-#endif
-- 
2.29.0

[PATCH v2 11/13] xen: remove GNUC check

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

QEMU requires Clang or GCC, that define and support __GNUC__ extensions

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/hw/xen/interface/io/ring.h | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/include/hw/xen/interface/io/ring.h b/include/hw/xen/interface/io/ring.h
index 5d048b335c..115705f3f4 100644
--- a/include/hw/xen/interface/io/ring.h
+++ b/include/hw/xen/interface/io/ring.h
@@ -206,21 +206,12 @@ typedef struct __name##_back_ring __name##_back_ring_t
 #define RING_HAS_UNCONSUMED_RESPONSES(_r)                               \
     ((_r)->sring->rsp_prod - (_r)->rsp_cons)
 
-#ifdef __GNUC__
 #define RING_HAS_UNCONSUMED_REQUESTS(_r) ({                             \
     unsigned int req = (_r)->sring->req_prod - (_r)->req_cons;          \
     unsigned int rsp = RING_SIZE(_r) -                                  \
         ((_r)->req_cons - (_r)->rsp_prod_pvt);                          \
     req < rsp ? req : rsp;                                              \
 })
-#else
-/* Same as above, but without the nice GCC ({ ... }) syntax. */
-#define RING_HAS_UNCONSUMED_REQUESTS(_r)                                \
-    ((((_r)->sring->req_prod - (_r)->req_cons) <                        \
-      (RING_SIZE(_r) - ((_r)->req_cons - (_r)->rsp_prod_pvt))) ?        \
-     ((_r)->sring->req_prod - (_r)->req_cons) :                         \
-     (RING_SIZE(_r) - ((_r)->req_cons - (_r)->rsp_prod_pvt)))
-#endif
 
 /* Direct access to individual ring elements, by index. */
 #define RING_GET_REQUEST(_r, _idx)                                      \
-- 
2.29.0

[PATCH v2 12/13] compiler: remove GNUC check

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

QEMU requires Clang or GCC, that define and support __GNUC__ extensions.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/compiler.h | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 108bfdb391..f492baf341 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -55,14 +55,10 @@
     (offsetof(container, field) + sizeof_field(container, field))
 
 /* Convert from a base type to a parent type, with compile time checking.  */
-#ifdef __GNUC__
 #define DO_UPCAST(type, field, dev) ( __extension__ ( { \
     char __attribute__((unused)) offset_must_be_zero[ \
         -offsetof(type, field)]; \
     container_of(dev, type, field);}))
-#else
-#define DO_UPCAST(type, field, dev) container_of(dev, type, field)
-#endif
 
 #define typeof_field(type, field) typeof(((type *)0)->field)
 #define type_check(t1,t2) ((t1*)0 - (t2*)0)
@@ -93,7 +89,7 @@
 #if defined(__clang__)
   /* clang doesn't support gnu_printf, so use printf. */
 # define GCC_FMT_ATTR(n, m) __attribute__((format(printf, n, m)))
-#elif defined(__GNUC__)
+#else
    /* Use gnu_printf (qemu uses standard format strings). */
 # define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
 # if defined(_WIN32)
@@ -101,8 +97,6 @@
     * even when MinGW or GLib include files use __printf__. */
 #  define __printf__ __gnu_printf__
 # endif
-#else
-#define GCC_FMT_ATTR(n, m)
 #endif
 
 #ifndef __has_warning
-- 
2.29.0

[PATCH v2 13/13] linux-user: remove GNUC check

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

QEMU requires Clang or GCC, that define and support __GNUC__ extensions.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 linux-user/strace.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 11fea14fba..e00275fcb5 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -24,7 +24,6 @@ struct syscallname {
                    abi_long, abi_long, abi_long);
 };
 
-#ifdef __GNUC__
 /*
  * It is possible that target doesn't have syscall that uses
  * following flags but we don't want the compiler to warn
@@ -32,9 +31,6 @@ struct syscallname {
  * functions.  It is ok to keep them while not used.
  */
 #define UNUSED __attribute__ ((unused))
-#else
-#define UNUSED
-#endif
 
 /*
  * Structure used to translate flag values into strings.  This is
-- 
2.29.0

Re: [PATCH v2 01/13] compiler.h: replace QEMU_GNUC_PREREQ macro

[Peter Maydell]

On Thu, 26 Nov 2020 at 11:29, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Replace it with glib G_GNUC_CHECK_VERSION.
>
> Available since 2.42, the macro received a small correction in
> 2.55.1 (glib d44afbadda8a "macros: make G_GNUC_CHECK_VERSION()
> portable" which was apparently harmless).
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qemu/atomic.h      |  2 +-
>  include/qemu/compiler.h    | 15 +++------------
>  scripts/cocci-macro-file.h |  1 -
>  accel/tcg/cpu-exec.c       |  2 +-
>  4 files changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
> index c1d211a351..c409257c11 100644
> --- a/include/qemu/atomic.h
> +++ b/include/qemu/atomic.h
> @@ -248,7 +248,7 @@
>   * 4.3 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36793).
>   */
>  #if defined(__i386__) || defined(__x86_64__)
> -#if !QEMU_GNUC_PREREQ(4, 4)
> +#if !G_GNUC_CHECK_VERSION(4, 4)
>  #if defined __x86_64__
>  #define smp_mb()    ({ asm volatile("mfence" ::: "memory"); (void)0; })
>  #else
> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> index c76281f354..cf044bab4a 100644
> --- a/include/qemu/compiler.h
> +++ b/include/qemu/compiler.h
> @@ -7,21 +7,12 @@
>  #ifndef COMPILER_H
>  #define COMPILER_H
>
> +#include "glib-compat.h"

osdep.h already includes glib-compat.h, so if it's safe to
include it this early we should just move that #include line
in osdep.h up above compiler.h. But I don't think it's going
to be quite that simple, because some parts of osdep.h
need to happen before we include any system headers, and
glib-compat.h includes some system headers. As it stands
this fragment will pull in glib-compat.h too early.

You probably need to rearrange osdep.h so it is
 * config-host.h
 * poison.h
 * bits that must go before any standard headers
 * pure system includes
 * glib-compat.h
 * compiler.h
(and hopefully none of the above needed stuff set up by compiler.h !)

Or if this tangle of bits depending on each other seems too
messy, we could just leave QEMU_GNUC_PREREQ the way it is :-)

thanks
-- PMM

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

[Peter Maydell]

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!)

thanks
-- PMM

Re: [PATCH v2 04/13] compiler.h: remove GCC < 3 __builtin_expect fallback

[Peter Maydell]

On Thu, 26 Nov 2020 at 11:29, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Since commit efc6c07 ("configure: Add a test for the minimum compiler
> version"), QEMU explicitely depends on GCC >= 4.8.
>
> (clang >= 3.8 advertizes itself as GCC >= 4.2 compatible)

All clangs always have advertised themselves as gcc-4.2-compatible,
as far as I'm aware. 3.4 is the version we need to care about,
I think it probably supported __builtin_expect(). (A test
of the whole series with gcc 4.8 and clang 3.4 would confirm this.)

thnaks
-- PMM

Re: [PATCH v2 01/13] compiler.h: replace QEMU_GNUC_PREREQ macro

[Marc-André Lureau]

Hi

On Thu, Nov 26, 2020 at 3:48 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 26 Nov 2020 at 11:29, <marcandre.lureau@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Replace it with glib G_GNUC_CHECK_VERSION.
> >
> > Available since 2.42, the macro received a small correction in
> > 2.55.1 (glib d44afbadda8a "macros: make G_GNUC_CHECK_VERSION()
> > portable" which was apparently harmless).
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  include/qemu/atomic.h      |  2 +-
> >  include/qemu/compiler.h    | 15 +++------------
> >  scripts/cocci-macro-file.h |  1 -
> >  accel/tcg/cpu-exec.c       |  2 +-
> >  4 files changed, 5 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
> > index c1d211a351..c409257c11 100644
> > --- a/include/qemu/atomic.h
> > +++ b/include/qemu/atomic.h
> > @@ -248,7 +248,7 @@
> >   * 4.3 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36793).
> >   */
> >  #if defined(__i386__) || defined(__x86_64__)
> > -#if !QEMU_GNUC_PREREQ(4, 4)
> > +#if !G_GNUC_CHECK_VERSION(4, 4)
> >  #if defined __x86_64__
> >  #define smp_mb()    ({ asm volatile("mfence" ::: "memory"); (void)0; })
> >  #else
> > diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> > index c76281f354..cf044bab4a 100644
> > --- a/include/qemu/compiler.h
> > +++ b/include/qemu/compiler.h
> > @@ -7,21 +7,12 @@
> >  #ifndef COMPILER_H
> >  #define COMPILER_H
> >
> > +#include "glib-compat.h"
>
> osdep.h already includes glib-compat.h, so if it's safe to
> include it this early we should just move that #include line
> in osdep.h up above compiler.h. But I don't think it's going
> to be quite that simple, because some parts of osdep.h
> need to happen before we include any system headers, and
> glib-compat.h includes some system headers. As it stands
> this fragment will pull in glib-compat.h too early.
>
> You probably need to rearrange osdep.h so it is
>  * config-host.h
>  * poison.h
>  * bits that must go before any standard headers
>  * pure system includes
>  * glib-compat.h
>  * compiler.h
> (and hopefully none of the above needed stuff set up by compiler.h !)
>
> Or if this tangle of bits depending on each other seems too
> messy, we could just leave QEMU_GNUC_PREREQ the way it is :-)

Or I just change the order the header are included in libvhost-user
(before https://patchew.org/QEMU/20201125100640.366523-1-marcandre.lureau@redhat.com/
lands), since that was the reason I didn't include osdep.h iirc.

Anyway, if the rest of the series is accepted, the include should go
away too (should have done that on top).

Re: [PATCH v2 05/13] qemu-plugin.h: remove GCC < 4

[Peter Maydell]

On Thu, 26 Nov 2020 at 11:30, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Since commit efc6c07 ("configure: Add a test for the minimum compiler
> version"), QEMU explicitely depends on GCC >= 4.8.
>
> (clang >= 3.8 advertizes itself as GCC >= 4.2 compatible)
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qemu/qemu-plugin.h | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)

This is an interesting case, because it's a header intended
for external use (people compiling plugins), not part of
QEMU proper. I don't know whether we want to impose the
same clang/gcc requirements on plugin builders, though it's
probably not a bad idea to do so. Alex ?

thanks
-- PMM

Re: [PATCH v2 00/13] Remove GCC < 4.8 checks

[no-reply]

Patchew URL: https://patchew.org/QEMU/20201126112915.525285-1-marcandre.lureau@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20201126112915.525285-1-marcandre.lureau@redhat.com
Subject: [PATCH v2 00/13] Remove GCC < 4.8 checks

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20201125185551.20475-1-berto@igalia.com -> patchew/20201125185551.20475-1-berto@igalia.com
 * [new tag]         patchew/20201126112915.525285-1-marcandre.lureau@redhat.com -> patchew/20201126112915.525285-1-marcandre.lureau@redhat.com
Switched to a new branch 'test'
77ab4c6 linux-user: remove GNUC check
f76a0b9 compiler: remove GNUC check
beeadb1 xen: remove GNUC check
405e99f poison: remove GNUC check
e5c6d98 audio: remove GNUC & MSVC check
2678b63 compiler.h: explicit case for Clang printf attribute
47e1c5e virtiofsd: replace _Static_assert with QEMU_BUILD_BUG_ON
5e15c4d tests: remove GCC < 4 fallbacks
a2ff6e8 qemu-plugin.h: remove GCC < 4
f0eebeb compiler.h: remove GCC < 3 __builtin_expect fallback
b157e9f accel/tcg: Remove special case for GCC < 4.6
93edd26 qemu/atomic: Drop special case for unsupported compiler
b5c4061 compiler.h: replace QEMU_GNUC_PREREQ macro

=== OUTPUT BEGIN ===
1/13 Checking commit b5c4061785ec (compiler.h: replace QEMU_GNUC_PREREQ macro)
WARNING: architecture specific defines should be avoided
#25: FILE: accel/tcg/cpu-exec.c:727:
+#if defined(__clang__) || !G_GNUC_CHECK_VERSION(4, 6)

total: 0 errors, 1 warnings, 54 lines checked

Patch 1/13 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/13 Checking commit 93edd26cd83a (qemu/atomic: Drop special case for unsupported compiler)
3/13 Checking commit b157e9f1f2df (accel/tcg: Remove special case for GCC < 4.6)
WARNING: architecture specific defines should be avoided
#30: FILE: accel/tcg/cpu-exec.c:727:
+#if defined(__clang__)

total: 0 errors, 1 warnings, 8 lines checked

Patch 3/13 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/13 Checking commit f0eebebebb3b (compiler.h: remove GCC < 3 __builtin_expect fallback)
5/13 Checking commit a2ff6e8ca3d5 (qemu-plugin.h: remove GCC < 4)
6/13 Checking commit 5e15c4d002dd (tests: remove GCC < 4 fallbacks)
ERROR: space prohibited between function name and open parenthesis '('
#28: FILE: tests/tcg/arm/fcvt.c:76:
+# define SNANF (__builtin_nansf (""))

ERROR: space prohibited between function name and open parenthesis '('
#29: FILE: tests/tcg/arm/fcvt.c:77:
+# define SNAN (__builtin_nans (""))

ERROR: space prohibited between function name and open parenthesis '('
#30: FILE: tests/tcg/arm/fcvt.c:78:
+# define SNANL (__builtin_nansl (""))

total: 3 errors, 0 warnings, 14 lines checked

Patch 6/13 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

7/13 Checking commit 47e1c5e0319a (virtiofsd: replace _Static_assert with QEMU_BUILD_BUG_ON)
8/13 Checking commit 2678b639f490 (compiler.h: explicit case for Clang printf attribute)
WARNING: architecture specific defines should be avoided
#37: FILE: include/qemu/compiler.h:93:
+#if defined(__clang__)

WARNING: Block comments use a leading /* on a separate line
#44: FILE: include/qemu/compiler.h:100:
+   /* Map __printf__ to __gnu_printf__ because we want standard format strings

WARNING: Block comments use a trailing */ on a separate line
#45: FILE: include/qemu/compiler.h:101:
+    * even when MinGW or GLib include files use __printf__. */

total: 0 errors, 3 warnings, 28 lines checked

Patch 8/13 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/13 Checking commit e5c6d9859362 (audio: remove GNUC & MSVC check)
10/13 Checking commit 405e99fe1fee (poison: remove GNUC check)
11/13 Checking commit beeadb186e66 (xen: remove GNUC check)
12/13 Checking commit f76a0b946371 (compiler: remove GNUC check)
13/13 Checking commit 77ab4c6a10d7 (linux-user: remove GNUC check)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20201126112915.525285-1-marcandre.lureau@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v2 06/13] tests: remove GCC < 4 fallbacks

[Peter Maydell]

On Thu, 26 Nov 2020 at 11:30, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Since commit efc6c07 ("configure: Add a test for the minimum compiler
> version"), QEMU explicitely depends on GCC >= 4.8.
>
> (clang >= 3.8 advertizes itself as GCC >= 4.2 compatible)
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/tcg/arm/fcvt.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/tests/tcg/arm/fcvt.c b/tests/tcg/arm/fcvt.c
> index 617626bc63..7ac47b564e 100644
> --- a/tests/tcg/arm/fcvt.c
> +++ b/tests/tcg/arm/fcvt.c
> @@ -73,11 +73,9 @@ static void print_int64(int i, int64_t num)
>
>  #ifndef SNANF
>  /* Signaling NaN macros, if supported.  */
> -# if __GNUC_PREREQ(3, 3)
> -#  define SNANF (__builtin_nansf (""))
> -#  define SNAN (__builtin_nans (""))
> -#  define SNANL (__builtin_nansl (""))
> -# endif
> +# define SNANF (__builtin_nansf (""))
> +# define SNAN (__builtin_nans (""))
> +# define SNANL (__builtin_nansl (""))
>  #endif

Please fix the commit message to talk about clang 3.4, not 3.8,
but otherwise

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH v2 08/13] compiler.h: explicit case for Clang printf attribute

[Peter Maydell]

On Thu, 26 Nov 2020 at 11:30, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Since commit efc6c07 ("configure: Add a test for the minimum compiler
> version"), QEMU explicitely depends on GCC >= 4.8, we could thus drop
> earlier version checks. Except clang advertizes itself as GCC 4.2.1.
>
> Since clang doesn't support gnu_printf, make that case explicitely and
> drop GCC version check.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qemu/compiler.h | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> index ae3e0df34c..108bfdb391 100644
> --- a/include/qemu/compiler.h
> +++ b/include/qemu/compiler.h
> @@ -90,18 +90,16 @@
>  #define QEMU_BUILD_BUG_ON_ZERO(x) (sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)) - \
>                                     sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)))
>
> -#if defined __GNUC__
> -# if !G_GNUC_CHECK_VERSION(4, 4)
> -   /* gcc versions before 4.4.x don't support gnu_printf, so use printf. */
> -#  define GCC_FMT_ATTR(n, m) __attribute__((format(printf, n, m)))
> -# else
> -   /* Use gnu_printf when supported (qemu uses standard format strings). */
> -#  define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
> -#  if defined(_WIN32)
> -    /* Map __printf__ to __gnu_printf__ because we want standard format strings
> -     * even when MinGW or GLib include files use __printf__. */
> -#   define __printf__ __gnu_printf__
> -#  endif
> +#if defined(__clang__)
> +  /* clang doesn't support gnu_printf, so use printf. */
> +# define GCC_FMT_ATTR(n, m) __attribute__((format(printf, n, m)))
> +#elif defined(__GNUC__)
> +   /* Use gnu_printf (qemu uses standard format strings). */
> +# define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
> +# if defined(_WIN32)
> +   /* Map __printf__ to __gnu_printf__ because we want standard format strings
> +    * even when MinGW or GLib include files use __printf__. */
> +#  define __printf__ __gnu_printf__
>  # endif

checkpatch probably warns about the block comment style so you might
as well fix it while touching that line, but otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH v2 09/13] audio: remove GNUC & MSVC check

[Peter Maydell]

On Thu, 26 Nov 2020 at 11:30, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> QEMU requires either GCC or Clang, which both advertize __GNUC__.
> Drop MSVC fallback path.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  audio/audio.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index 46578e4a58..d7a00294de 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -122,13 +122,7 @@ int audio_bug (const char *funcname, int cond)
>
>  #if defined AUDIO_BREAKPOINT_ON_BUG
>  #  if defined HOST_I386
> -#    if defined __GNUC__
> -        __asm__ ("int3");
> -#    elif defined _MSC_VER
> -        _asm _emit 0xcc;
> -#    else
> -        abort ();
> -#    endif
> +      __asm__ ("int3");
>  #  else
>          abort ();
>  #  endif
> --
> 2.29.0

I would prefer to just drop this attempt to emit an inline
breakpoint insn (which won't work on non-x86 hosts and seems
to me to have no benefit over just calling abort(), which will
also drop you into the debugger) and simply make it abort() if
AUDIO_BREAKPOINT_ON_BUG is defined. Gerd, do you have an
opinion ?

thanks
-- PMM

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

[Daniel P. Berrangé]

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.

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

Re: [PATCH v2 10/13] poison: remove GNUC check

[Peter Maydell]

On Thu, 26 Nov 2020 at 11:30, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> QEMU requires Clang or GCC, that define and support __GNUC__ extensions
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/exec/poison.h | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH v2 11/13] xen: remove GNUC check

[Peter Maydell]

On Thu, 26 Nov 2020 at 11:30, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> QEMU requires Clang or GCC, that define and support __GNUC__ extensions
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/hw/xen/interface/io/ring.h | 9 ---------
>  1 file changed, 9 deletions(-)

From QEMU's POV this change is good, but this header seems to have
originally been an import from the Xen public headers -- are we
happy to diverge from that original or do we prefer to stay as
close as possible to the upstream header? Cc'ing the Xen maintainers
for their opinion.

> diff --git a/include/hw/xen/interface/io/ring.h b/include/hw/xen/interface/io/ring.h
> index 5d048b335c..115705f3f4 100644
> --- a/include/hw/xen/interface/io/ring.h
> +++ b/include/hw/xen/interface/io/ring.h
> @@ -206,21 +206,12 @@ typedef struct __name##_back_ring __name##_back_ring_t
>  #define RING_HAS_UNCONSUMED_RESPONSES(_r)                               \
>      ((_r)->sring->rsp_prod - (_r)->rsp_cons)
>
> -#ifdef __GNUC__
>  #define RING_HAS_UNCONSUMED_REQUESTS(_r) ({                             \
>      unsigned int req = (_r)->sring->req_prod - (_r)->req_cons;          \
>      unsigned int rsp = RING_SIZE(_r) -                                  \
>          ((_r)->req_cons - (_r)->rsp_prod_pvt);                          \
>      req < rsp ? req : rsp;                                              \
>  })
> -#else
> -/* Same as above, but without the nice GCC ({ ... }) syntax. */
> -#define RING_HAS_UNCONSUMED_REQUESTS(_r)                                \
> -    ((((_r)->sring->req_prod - (_r)->req_cons) <                        \
> -      (RING_SIZE(_r) - ((_r)->req_cons - (_r)->rsp_prod_pvt))) ?        \
> -     ((_r)->sring->req_prod - (_r)->req_cons) :                         \
> -     (RING_SIZE(_r) - ((_r)->req_cons - (_r)->rsp_prod_pvt)))
> -#endif
>
>  /* Direct access to individual ring elements, by index. */
>  #define RING_GET_REQUEST(_r, _idx)                                      \
> --


thanks
-- PMM

Re: [PATCH v2 09/13] audio: remove GNUC & MSVC check

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

Hi

On Thu, Nov 26, 2020 at 4:08 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Thu, 26 Nov 2020 at 11:30, <marcandre.lureau@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > QEMU requires either GCC or Clang, which both advertize __GNUC__.
> > Drop MSVC fallback path.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  audio/audio.c | 8 +-------
> >  1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/audio/audio.c b/audio/audio.c
> > index 46578e4a58..d7a00294de 100644
> > --- a/audio/audio.c
> > +++ b/audio/audio.c
> > @@ -122,13 +122,7 @@ int audio_bug (const char *funcname, int cond)
> >
> >  #if defined AUDIO_BREAKPOINT_ON_BUG
> >  #  if defined HOST_I386
> > -#    if defined __GNUC__
> > -        __asm__ ("int3");
> > -#    elif defined _MSC_VER
> > -        _asm _emit 0xcc;
> > -#    else
> > -        abort ();
> > -#    endif
> > +      __asm__ ("int3");
> >  #  else
> >          abort ();
> >  #  endif
> > --
> > 2.29.0
>
> I would prefer to just drop this attempt to emit an inline
> breakpoint insn (which won't work on non-x86 hosts and seems
> to me to have no benefit over just calling abort(), which will
> also drop you into the debugger) and simply make it abort() if
> AUDIO_BREAKPOINT_ON_BUG is defined. Gerd, do you have an
> opinion ?
>
>
Oh yeah.. But audio/ is a rabbit hole, so I had to refrain myself doing
more cleanups.

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2374 bytes --]

Re: [PATCH v2 12/13] compiler: remove GNUC check

[Peter Maydell]

On Thu, 26 Nov 2020 at 11:30, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> QEMU requires Clang or GCC, that define and support __GNUC__ extensions.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qemu/compiler.h | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> index 108bfdb391..f492baf341 100644
> --- a/include/qemu/compiler.h
> +++ b/include/qemu/compiler.h
> @@ -55,14 +55,10 @@
>      (offsetof(container, field) + sizeof_field(container, field))
>
>  /* Convert from a base type to a parent type, with compile time checking.  */
> -#ifdef __GNUC__
>  #define DO_UPCAST(type, field, dev) ( __extension__ ( { \
>      char __attribute__((unused)) offset_must_be_zero[ \
>          -offsetof(type, field)]; \
>      container_of(dev, type, field);}))
> -#else
> -#define DO_UPCAST(type, field, dev) container_of(dev, type, field)
> -#endif

This bit looks good.

>  #define typeof_field(type, field) typeof(((type *)0)->field)
>  #define type_check(t1,t2) ((t1*)0 - (t2*)0)
> @@ -93,7 +89,7 @@
>  #if defined(__clang__)
>    /* clang doesn't support gnu_printf, so use printf. */
>  # define GCC_FMT_ATTR(n, m) __attribute__((format(printf, n, m)))
> -#elif defined(__GNUC__)
> +#else
>     /* Use gnu_printf (qemu uses standard format strings). */
>  # define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
>  # if defined(_WIN32)
> @@ -101,8 +97,6 @@
>      * even when MinGW or GLib include files use __printf__. */
>  #  define __printf__ __gnu_printf__
>  # endif
> -#else
> -#define GCC_FMT_ATTR(n, m)
>  #endif

Didn't you already change the GCC_FMT_ATTR stuff in a previous
patch in the series? If so this part should just be squashed
into that one.

thanks
-- PMM

Re: [PATCH v2 13/13] linux-user: remove GNUC check

[Peter Maydell]

On Thu, 26 Nov 2020 at 11:31, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> QEMU requires Clang or GCC, that define and support __GNUC__ extensions.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH v2 05/13] qemu-plugin.h: remove GCC < 4

[Daniel P. Berrangé]

On Thu, Nov 26, 2020 at 11:57:14AM +0000, Peter Maydell wrote:
> On Thu, 26 Nov 2020 at 11:30, <marcandre.lureau@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Since commit efc6c07 ("configure: Add a test for the minimum compiler
> > version"), QEMU explicitely depends on GCC >= 4.8.
> >
> > (clang >= 3.8 advertizes itself as GCC >= 4.2 compatible)
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  include/qemu/qemu-plugin.h | 9 ++-------
> >  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> This is an interesting case, because it's a header intended
> for external use (people compiling plugins), not part of
> QEMU proper. I don't know whether we want to impose the
> same clang/gcc requirements on plugin builders, though it's
> probably not a bad idea to do so. Alex ?

IIUC, we expect that the plugins are built specifically to go with
the QEMU you have built. With that in mind, it looks reasonable to
assume the same compiler toolchain versions for both.

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

Re: [PATCH v2 12/13] compiler: remove GNUC check

[Marc-André Lureau]

On Thu, Nov 26, 2020 at 4:10 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 26 Nov 2020 at 11:30, <marcandre.lureau@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > QEMU requires Clang or GCC, that define and support __GNUC__ extensions.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  include/qemu/compiler.h | 8 +-------
> >  1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> > index 108bfdb391..f492baf341 100644
> > --- a/include/qemu/compiler.h
> > +++ b/include/qemu/compiler.h
> > @@ -55,14 +55,10 @@
> >      (offsetof(container, field) + sizeof_field(container, field))
> >
> >  /* Convert from a base type to a parent type, with compile time checking.  */
> > -#ifdef __GNUC__
> >  #define DO_UPCAST(type, field, dev) ( __extension__ ( { \
> >      char __attribute__((unused)) offset_must_be_zero[ \
> >          -offsetof(type, field)]; \
> >      container_of(dev, type, field);}))
> > -#else
> > -#define DO_UPCAST(type, field, dev) container_of(dev, type, field)
> > -#endif
>
> This bit looks good.
>
> >  #define typeof_field(type, field) typeof(((type *)0)->field)
> >  #define type_check(t1,t2) ((t1*)0 - (t2*)0)
> > @@ -93,7 +89,7 @@
> >  #if defined(__clang__)
> >    /* clang doesn't support gnu_printf, so use printf. */
> >  # define GCC_FMT_ATTR(n, m) __attribute__((format(printf, n, m)))
> > -#elif defined(__GNUC__)
> > +#else
> >     /* Use gnu_printf (qemu uses standard format strings). */
> >  # define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
> >  # if defined(_WIN32)
> > @@ -101,8 +97,6 @@
> >      * even when MinGW or GLib include files use __printf__. */
> >  #  define __printf__ __gnu_printf__
> >  # endif
> > -#else
> > -#define GCC_FMT_ATTR(n, m)
> >  #endif
>
> Didn't you already change the GCC_FMT_ATTR stuff in a previous
> patch in the series? If so this part should just be squashed
> into that one.
>

It's a mix of concerns. I focused on the first patch on the clang
exception, then dropped the explicit GNUC check. I would rather keep
it that way.

thanks

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

[Peter Maydell]

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.

thanks
-- PMM

Re: [PATCH v2 05/13] qemu-plugin.h: remove GCC < 4

[Peter Maydell]

On Thu, 26 Nov 2020 at 12:12, Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Thu, Nov 26, 2020 at 11:57:14AM +0000, Peter Maydell wrote:
> > This is an interesting case, because it's a header intended
> > for external use (people compiling plugins), not part of
> > QEMU proper. I don't know whether we want to impose the
> > same clang/gcc requirements on plugin builders, though it's
> > probably not a bad idea to do so. Alex ?
>
> IIUC, we expect that the plugins are built specifically to go with
> the QEMU you have built.

No, the point of the plugin interface is that it actually is
a (constrained) stable ABI with version-querying and checking
so you don't have to build a plugin against the exact matching
QEMU version. (But "don't use really ancient gcc" doesn't seem
like a very major thing to ask of its users.)

thanks
-- PMM

Re: [PATCH v2 05/13] qemu-plugin.h: remove GCC < 4

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Thu, 26 Nov 2020 at 12:12, Daniel P. Berrangé <berrange@redhat.com> wrote:
>> On Thu, Nov 26, 2020 at 11:57:14AM +0000, Peter Maydell wrote:
>> > This is an interesting case, because it's a header intended
>> > for external use (people compiling plugins), not part of
>> > QEMU proper. I don't know whether we want to impose the
>> > same clang/gcc requirements on plugin builders, though it's
>> > probably not a bad idea to do so. Alex ?
>>
>> IIUC, we expect that the plugins are built specifically to go with
>> the QEMU you have built.
>
> No, the point of the plugin interface is that it actually is
> a (constrained) stable ABI with version-querying and checking
> so you don't have to build a plugin against the exact matching
> QEMU version. (But "don't use really ancient gcc" doesn't seem
> like a very major thing to ask of its users.)

Quite. But I agree using a GCC from this decade seems reasonable
especially as the feature has only been in QEMU for a few cycles.

Acked-by: Alex Bennée <alex.bennee@linaro.org>
          
-- 
Alex Bennée

Re: [PATCH v2 09/13] audio: remove GNUC & MSVC check

[Gerd Hoffmann]

  Hi,

> >  #if defined AUDIO_BREAKPOINT_ON_BUG
> >  #  if defined HOST_I386
> > +      __asm__ ("int3");
> >  #  else
> >          abort ();
> >  #  endif
> > --
> > 2.29.0
> 
> I would prefer to just drop this attempt to emit an inline
> breakpoint insn (which won't work on non-x86 hosts and seems
> to me to have no benefit over just calling abort(), which will
> also drop you into the debugger) and simply make it abort() if
> AUDIO_BREAKPOINT_ON_BUG is defined. Gerd, do you have an
> opinion ?

   kraxel@sirius ~/projects/qemu# git grep AUDIO_BREAKPOINT_ON_BUG
   audio/audio.c:#if defined AUDIO_BREAKPOINT_ON_BUG

Seems this is unused, so just remove it?

take care,
  Gerd

Re: [PATCH v2 09/13] audio: remove GNUC & MSVC check

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

On Fri, Nov 27, 2020 at 11:15 AM Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
>
> > >  #if defined AUDIO_BREAKPOINT_ON_BUG
> > >  #  if defined HOST_I386
> > > +      __asm__ ("int3");
> > >  #  else
> > >          abort ();
> > >  #  endif
> > > --
> > > 2.29.0
> >
> > I would prefer to just drop this attempt to emit an inline
> > breakpoint insn (which won't work on non-x86 hosts and seems
> > to me to have no benefit over just calling abort(), which will
> > also drop you into the debugger) and simply make it abort() if
> > AUDIO_BREAKPOINT_ON_BUG is defined. Gerd, do you have an
> > opinion ?
>
>    kraxel@sirius ~/projects/qemu# git grep AUDIO_BREAKPOINT_ON_BUG
>    audio/audio.c:#if defined AUDIO_BREAKPOINT_ON_BUG
>
> Seems this is unused, so just remove it?
>
>
I think the original intention was to build with CFLAGS set manually for
debugging this limited case. (Iirc, we have other such #ifdef lurking
around)

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 1527 bytes --]

Re: [PATCH v2 06/13] tests: remove GCC < 4 fallbacks

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Thu, 26 Nov 2020 at 11:30, <marcandre.lureau@redhat.com> wrote:
>>
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Since commit efc6c07 ("configure: Add a test for the minimum compiler
>> version"), QEMU explicitely depends on GCC >= 4.8.

technically the tests/tcg tests don't depend on the same compilers as
the main QEMU build. However as the cross compiler case is most likely
covered by our much more modern docker images and the buildin arm-on-arm
will re-use the host compiler this is fine.

Acked-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

Re: [PATCH v2 11/13] xen: remove GNUC check

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 2258 bytes --]

On Thu, 26 Nov 2020, Peter Maydell wrote:
> On Thu, 26 Nov 2020 at 11:30, <marcandre.lureau@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > QEMU requires Clang or GCC, that define and support __GNUC__ extensions
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  include/hw/xen/interface/io/ring.h | 9 ---------
> >  1 file changed, 9 deletions(-)
> 
> >From QEMU's POV this change is good, but this header seems to have
> originally been an import from the Xen public headers -- are we
> happy to diverge from that original or do we prefer to stay as
> close as possible to the upstream header? Cc'ing the Xen maintainers
> for their opinion.

Small divergences are not a problem. I think this is OK.


> > diff --git a/include/hw/xen/interface/io/ring.h b/include/hw/xen/interface/io/ring.h
> > index 5d048b335c..115705f3f4 100644
> > --- a/include/hw/xen/interface/io/ring.h
> > +++ b/include/hw/xen/interface/io/ring.h
> > @@ -206,21 +206,12 @@ typedef struct __name##_back_ring __name##_back_ring_t
> >  #define RING_HAS_UNCONSUMED_RESPONSES(_r)                               \
> >      ((_r)->sring->rsp_prod - (_r)->rsp_cons)
> >
> > -#ifdef __GNUC__
> >  #define RING_HAS_UNCONSUMED_REQUESTS(_r) ({                             \
> >      unsigned int req = (_r)->sring->req_prod - (_r)->req_cons;          \
> >      unsigned int rsp = RING_SIZE(_r) -                                  \
> >          ((_r)->req_cons - (_r)->rsp_prod_pvt);                          \
> >      req < rsp ? req : rsp;                                              \
> >  })
> > -#else
> > -/* Same as above, but without the nice GCC ({ ... }) syntax. */
> > -#define RING_HAS_UNCONSUMED_REQUESTS(_r)                                \
> > -    ((((_r)->sring->req_prod - (_r)->req_cons) <                        \
> > -      (RING_SIZE(_r) - ((_r)->req_cons - (_r)->rsp_prod_pvt))) ?        \
> > -     ((_r)->sring->req_prod - (_r)->req_cons) :                         \
> > -     (RING_SIZE(_r) - ((_r)->req_cons - (_r)->rsp_prod_pvt)))
> > -#endif
> >
> >  /* Direct access to individual ring elements, by index. */
> >  #define RING_GET_REQUEST(_r, _idx)                                      \

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

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 2115 bytes --]

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


-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 3067 bytes --]

Re: [PATCH v2 04/13] compiler.h: remove GCC < 3 __builtin_expect fallback

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]

Hi

On Thu, Nov 26, 2020 at 3:57 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Thu, 26 Nov 2020 at 11:29, <marcandre.lureau@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Since commit efc6c07 ("configure: Add a test for the minimum compiler
> > version"), QEMU explicitely depends on GCC >= 4.8.
> >
> > (clang >= 3.8 advertizes itself as GCC >= 4.2 compatible)
>
> All clangs always have advertised themselves as gcc-4.2-compatible,
> as far as I'm aware. 3.4 is the version we need to care about,
> I think it probably supported __builtin_expect(). (A test
> of the whole series with gcc 4.8 and clang 3.4 would confirm this.)
>

clang 3.4 also advertizes as gcc-4.2, thus we can safely remove that
__builtin_expect() fallback (if it didn't have it, it would have failed for
a long while)

I also checked the __builtin_expect() support with clang-3.4.2-9.el7.x86_64
from EPEL7 successfully.




-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 1648 bytes --]

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

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

[Daniel P. Berrangé]

On Thu, Dec 10, 2020 at 01:42:51PM +0000, Daniel P. Berrangé wrote:
> 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

Actually, since you've validated main clang 3.4 code, you can assume
this applies equally to xcode 5.1, as that is using the clang 3.4 codebase.

Basically we can assume features match across the regular and xcode
clangs, with the mapped versions. IOW changing either is sufficient,
no need to check both IMHO.

  https://en.wikipedia.org/wiki/Xcode#Xcode_5.0_-_6.x_(since_arm64_support)_2

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