diff mbox series

[2/3] qemu/atomic: Drop special case for unsupported compiler

Message ID 20200928125859.734287-3-philmd@redhat.com
State New
Headers show
Series qemu/compiler: Remove unused special case code for GCC < 4.8 | expand

Commit Message

Philippe Mathieu-Daudé Sept. 28, 2020, 12:58 p.m. UTC
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").

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

Comments

Peter Maydell Sept. 28, 2020, 1:36 p.m. UTC | #1
On Mon, 28 Sep 2020 at 14:12, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> 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").

> -/*
> - * 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 !QEMU_GNUC_PREREQ(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

You need to be a bit cautious about removing QEMU_GNUC_PREREQ()
checks, because clang advertises itself as GCC 4.2 via the
__GNUC__ and __GNUC_MINOR__ macros that QEMU_GNUC_PREREQ()
tests, and so unless the code has explicitly handled clang
via a different ifdef or feature test clang will be using
the fallback codepath in cases like this. So you also need
to find out whether all our supported versions of clang
are OK with the gcc-4.4-and-up version of this code...
(I think that deleting this set of #ifs means we end up
with the "just use __sync_synchronize()" version of smp_mb()
later on in the header?)

thanks
-- PMM
diff mbox series

Patch

diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index c1d211a3519..8f4b3a80fbd 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 !QEMU_GNUC_PREREQ(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