On Mon, Oct 16, 2017 at 05:08:34PM -0700, Yi-Hung Wei wrote:
> Thanks for the patch. I can verify that it works great with C++14 support.
>
> One minor problem is that it may run into some compiler errors if the
> C++ compiler does not support C++14 or the support is not enabled.
> There was a patch that hit similar issue:
> https://patchwork.ozlabs.org/patch/813112/
This is a good point. What if we fold in the following incremental?
I'm also appending the full patch.
diff --git a/lib/ovs-atomic.h b/lib/ovs-atomic.h
index 5e4a9c1775b3..c5299fd08512 100644
--- a/lib/ovs-atomic.h
+++ b/lib/ovs-atomic.h
@@ -325,11 +325,11 @@
#include "ovs-atomic-pthreads.h"
#elif __has_extension(c_atomic)
#include "ovs-atomic-clang.h"
- #elif defined(__cplusplus) && HAVE_ATOMIC
+ #elif HAVE_ATOMIC && __cplusplus >= 201103L
#include "ovs-atomic-c++.h"
- #elif HAVE_STDATOMIC_H
+ #elif HAVE_STDATOMIC_H && !defined(__cplusplus)
#include "ovs-atomic-c11.h"
- #elif __GNUC__ >= 5
+ #elif __GNUC__ >= 5 && !defined(__cplusplus)
#error "GCC 5+ should have <stdatomic.h>"
#elif __GNUC__ >= 4 && __GNUC_MINOR__ >= 7
#include "ovs-atomic-gcc4.7+.h"
--8<--------------------------cut here-------------------------->8--
From: Ben Pfaff <[email protected]>
Date: Tue, 17 Oct 2017 13:34:29 -0700
Subject: [PATCH] ovs-atomic: Add C++ compatible implementation.
G++ 5 does not implement the _Atomic keyword, which is part of C11 but not
C++14, so the existing <stdatomic.h> based atomic implementation doesn't
work. This commit adds a new implementation based on the C++14 <atomic>
header.
In this area, C++ is pickier about types than C, so a few of the
definitions in ovs-atomic.h have to be updated to use more precise types
for integer constants.
This updates the code that generates cxxtest.cc to #include <config.h>
(so that HAVE_ATOMIC is defined) and to automatically regenerate when the
program is reconfigured (because otherwise the #include <config.h>) won't
get added without a "make clean" step).
"ovs-atomic.h" is not a public header, but apparently some code was
using it anyway.
Fixes: 9c463631e8145 ("ovs-atomic: Report error for contradictory
configuration.")
Reported-by: Yi-Hung Wei <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
---
include/openvswitch/automake.mk | 10 ++++---
lib/automake.mk | 1 +
lib/ovs-atomic-c++.h | 64 +++++++++++++++++++++++++++++++++++++++++
lib/ovs-atomic.h | 18 ++++++------
m4/openvswitch.m4 | 3 ++
5 files changed, 84 insertions(+), 12 deletions(-)
create mode 100644 lib/ovs-atomic-c++.h
diff --git a/include/openvswitch/automake.mk b/include/openvswitch/automake.mk
index eabeb1e971f3..6cb19b84e220 100644
--- a/include/openvswitch/automake.mk
+++ b/include/openvswitch/automake.mk
@@ -41,10 +41,12 @@ if HAVE_CXX
# are acceptable as C++.
noinst_LTLIBRARIES += include/openvswitch/libcxxtest.la
nodist_include_openvswitch_libcxxtest_la_SOURCES =
include/openvswitch/cxxtest.cc
-include/openvswitch/cxxtest.cc: include/openvswitch/automake.mk
- $(AM_V_GEN)for header in $(openvswitchinclude_HEADERS); do \
- echo $$header; \
- done | sed 's,^include/\(.*\)$$,#include <\1>,' > $@
+include/openvswitch/cxxtest.cc: \
+ include/openvswitch/automake.mk $(top_builddir)/config.status
+ $(AM_V_GEN){ echo "#include <config.h>"; \
+ for header in $(openvswitchinclude_HEADERS); do \
+ echo $$header; \
+ done | sed 's,^include/\(.*\)$$,#include <\1>,'; } > $@
endif
# OVS does not use C++ itself, but it provides public header files
diff --git a/lib/automake.mk b/lib/automake.mk
index 2415f4cd6c25..ca1cf5dd2524 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -159,6 +159,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/ofp-version-opt.h \
lib/ofp-version-opt.c \
lib/ofpbuf.c \
+ lib/ovs-atomic-c++.h \
lib/ovs-atomic-c11.h \
lib/ovs-atomic-clang.h \
lib/ovs-atomic-flag-gcc4.7+.h \
diff --git a/lib/ovs-atomic-c++.h b/lib/ovs-atomic-c++.h
new file mode 100644
index 000000000000..5bb88536d799
--- /dev/null
+++ b/lib/ovs-atomic-c++.h
@@ -0,0 +1,64 @@
+/* This header implements atomic operation primitives on compilers that
+ * have built-in support for C11 <stdatomic.h> */
+#ifndef IN_OVS_ATOMIC_H
+#error "This header should only be included indirectly via ovs-atomic.h."
+#endif
+
+#include <atomic>
+
+#define ATOMIC(TYPE) std::atomic<TYPE>
+
+using std::atomic_init;
+
+using std::memory_order_relaxed;
+using std::memory_order_consume;
+using std::memory_order_acquire;
+using std::memory_order_release;
+using std::memory_order_acq_rel;
+using std::memory_order_seq_cst;
+
+using std::atomic_thread_fence;
+using std::atomic_signal_fence;
+using std::atomic_is_lock_free;
+
+using std::atomic_store;
+using std::atomic_store_explicit;
+
+using std::atomic_compare_exchange_strong;
+using std::atomic_compare_exchange_strong_explicit;
+using std::atomic_compare_exchange_weak;
+using std::atomic_compare_exchange_weak_explicit;
+
+#define atomic_read(SRC, DST) \
+ atomic_read_explicit(SRC, DST, memory_order_seq_cst)
+#define atomic_read_explicit(SRC, DST, ORDER) \
+ (*(DST) = std::atomic_load_explicit(SRC, ORDER), \
+ (void) 0)
+
+#define atomic_add(RMW, ARG, ORIG) \
+ atomic_add_explicit(RMW, ARG, ORIG, memory_order_seq_cst)
+#define atomic_sub(RMW, ARG, ORIG) \
+ atomic_sub_explicit(RMW, ARG, ORIG, memory_order_seq_cst)
+#define atomic_or(RMW, ARG, ORIG) \
+ atomic_or_explicit(RMW, ARG, ORIG, memory_order_seq_cst)
+#define atomic_xor(RMW, ARG, ORIG) \
+ atomic_xor_explicit(RMW, ARG, ORIG, memory_order_seq_cst)
+#define atomic_and(RMW, ARG, ORIG) \
+ atomic_and_explicit(RMW, ARG, ORIG, memory_order_seq_cst)
+
+#define atomic_add_explicit(RMW, ARG, ORIG, ORDER) \
+ (*(ORIG) = std::atomic_fetch_add_explicit(RMW, ARG, ORDER), (void) 0)
+#define atomic_sub_explicit(RMW, ARG, ORIG, ORDER) \
+ (*(ORIG) = std::atomic_fetch_sub_explicit(RMW, ARG, ORDER), (void) 0)
+#define atomic_or_explicit(RMW, ARG, ORIG, ORDER) \
+ (*(ORIG) = std::atomic_fetch_or_explicit(RMW, ARG, ORDER), (void) 0)
+#define atomic_xor_explicit(RMW, ARG, ORIG, ORDER) \
+ (*(ORIG) = std::atomic_fetch_xor_explicit(RMW, ARG, ORDER), (void) 0)
+#define atomic_and_explicit(RMW, ARG, ORIG, ORDER) \
+ (*(ORIG) = std::atomic_fetch_and_explicit(RMW, ARG, ORDER), (void) 0)
+
+using std::atomic_flag;
+using std::atomic_flag_test_and_set_explicit;
+using std::atomic_flag_test_and_set;
+using std::atomic_flag_clear_explicit;
+using std::atomic_flag_clear;
diff --git a/lib/ovs-atomic.h b/lib/ovs-atomic.h
index d1b4e09e70d4..c5299fd08512 100644
--- a/lib/ovs-atomic.h
+++ b/lib/ovs-atomic.h
@@ -325,9 +325,11 @@
#include "ovs-atomic-pthreads.h"
#elif __has_extension(c_atomic)
#include "ovs-atomic-clang.h"
- #elif HAVE_STDATOMIC_H
+ #elif HAVE_ATOMIC && __cplusplus >= 201103L
+ #include "ovs-atomic-c++.h"
+ #elif HAVE_STDATOMIC_H && !defined(__cplusplus)
#include "ovs-atomic-c11.h"
- #elif __GNUC__ >= 5
+ #elif __GNUC__ >= 5 && !defined(__cplusplus)
#error "GCC 5+ should have <stdatomic.h>"
#elif __GNUC__ >= 4 && __GNUC_MINOR__ >= 7
#include "ovs-atomic-gcc4.7+.h"
@@ -446,7 +448,7 @@ atomic_count_inc(atomic_count *count)
{
unsigned int old;
- atomic_add_relaxed(&count->count, 1, &old);
+ atomic_add_relaxed(&count->count, 1u, &old);
return old;
}
@@ -456,7 +458,7 @@ atomic_count_dec(atomic_count *count)
{
unsigned int old;
- atomic_sub_relaxed(&count->count, 1, &old);
+ atomic_sub_relaxed(&count->count, 1u, &old);
return old;
}
@@ -486,7 +488,7 @@ struct ovs_refcount {
static inline void
ovs_refcount_init(struct ovs_refcount *refcount)
{
- atomic_init(&refcount->count, 1);
+ atomic_init(&refcount->count, 1u);
}
/* Increments 'refcount'.
@@ -498,7 +500,7 @@ ovs_refcount_ref(struct ovs_refcount *refcount)
{
unsigned int old_refcount;
- atomic_add_explicit(&refcount->count, 1, &old_refcount,
+ atomic_add_explicit(&refcount->count, 1u, &old_refcount,
memory_order_relaxed);
ovs_assert(old_refcount > 0);
}
@@ -521,7 +523,7 @@ ovs_refcount_unref(struct ovs_refcount *refcount)
{
unsigned int old_refcount;
- atomic_sub_explicit(&refcount->count, 1, &old_refcount,
+ atomic_sub_explicit(&refcount->count, 1u, &old_refcount,
memory_order_release);
ovs_assert(old_refcount > 0);
if (old_refcount == 1) {
@@ -619,7 +621,7 @@ ovs_refcount_unref_relaxed(struct ovs_refcount *refcount)
{
unsigned int old_refcount;
- atomic_sub_explicit(&refcount->count, 1, &old_refcount,
+ atomic_sub_explicit(&refcount->count, 1u, &old_refcount,
memory_order_relaxed);
ovs_assert(old_refcount > 0);
return old_refcount;
diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
index 00ffad35f6b0..59e1352e3cf5 100644
--- a/m4/openvswitch.m4
+++ b/m4/openvswitch.m4
@@ -632,6 +632,9 @@ AC_DEFUN([OVS_CHECK_CXX],
AX_CXX_COMPILE_STDCXX([11], [], [optional])
if test $enable_Werror = yes && test $HAVE_CXX11 = 1; then
enable_cxx=:
+ AC_LANG_PUSH([C++])
+ AC_CHECK_HEADERS([atomic])
+ AC_LANG_POP([C++])
else
enable_cxx=false
fi
--
2.10.2
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
