[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kevin Hu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL292184: [Test patch] Inline hot functions in libcxx 
shared_ptr (authored by hxy9243).

Changed prior to commit:
  https://reviews.llvm.org/D24991?vs=84569=84622#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24991

Files:
  libcxx/trunk/include/memory
  libcxx/trunk/src/memory.cpp

Index: libcxx/trunk/src/memory.cpp
===
--- libcxx/trunk/src/memory.cpp
+++ libcxx/trunk/src/memory.cpp
@@ -17,28 +17,6 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-namespace
-{
-
-// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
-// should be sufficient for thread safety.
-// See https://llvm.org/bugs/show_bug.cgi?id=22803
-template 
-inline T
-increment(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, 1, _AO_Relaxed);
-}
-
-template 
-inline T
-decrement(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, -1, _AO_Acq_Rel);
-}
-
-}  // namespace
-
 const allocator_arg_t allocator_arg = allocator_arg_t();
 
 bad_weak_ptr::~bad_weak_ptr() _NOEXCEPT {}
@@ -53,27 +31,27 @@
 {
 }
 
+__shared_weak_count::~__shared_weak_count()
+{
+}
+
 void
 __shared_count::__add_shared() _NOEXCEPT
 {
-increment(__shared_owners_);
+__libcpp_atomic_refcount_increment(__shared_owners_);
 }
 
 bool
 __shared_count::__release_shared() _NOEXCEPT
 {
-if (decrement(__shared_owners_) == -1)
+if (__libcpp_atomic_refcount_decrement(__shared_owners_) == -1)
 {
 __on_zero_shared();
 return true;
 }
 return false;
 }
 
-__shared_weak_count::~__shared_weak_count()
-{
-}
-
 void
 __shared_weak_count::__add_shared() _NOEXCEPT
 {
@@ -83,7 +61,7 @@
 void
 __shared_weak_count::__add_weak() _NOEXCEPT
 {
-increment(__shared_weak_owners_);
+__libcpp_atomic_refcount_increment(__shared_weak_owners_);
 }
 
 void
@@ -124,7 +102,7 @@
 //__libcpp_atomic_store(&__shared_weak_owners_, -1, _AO_Release);
 __on_zero_shared_weak();
 }
-else if (decrement(__shared_weak_owners_) == -1)
+else if (__libcpp_atomic_refcount_decrement(__shared_weak_owners_) == -1)
 __on_zero_shared_weak();
 }
 
Index: libcxx/trunk/include/memory
===
--- libcxx/trunk/include/memory
+++ libcxx/trunk/include/memory
@@ -3681,6 +3681,39 @@
 
 #endif // _LIBCPP_STD_VER > 14
 
+// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
+// should be sufficient for thread safety.
+// See https://llvm.org/bugs/show_bug.cgi?id=22803
+#if defined(__clang__) && __has_builtin(__atomic_add_fetch)  \
+   && defined(__ATOMIC_RELAXED)  \
+   && defined(__ATOMIC_ACQ_REL)
+#   define _LIBCPP_HAS_BUILTIN_ATOMIC_SUPPORT
+#elif !defined(__clang__) && defined(_GNUC_VER) && _GNUC_VER >= 407
+#   define _LIBCPP_HAS_BUILTIN_ATOMIC_SUPPORT
+#endif
+
+template 
+inline _LIBCPP_INLINE_VISIBILITY _Tp
+__libcpp_atomic_refcount_increment(_Tp& __t) _NOEXCEPT
+{
+#if defined(_LIBCPP_HAS_BUILTIN_ATOMIC_SUPPORT) && !defined(_LIBCPP_HAS_NO_THREADS)
+return __atomic_add_fetch(&__t, 1, __ATOMIC_RELAXED);
+#else
+return __t += 1;
+#endif
+}
+
+template 
+inline _LIBCPP_INLINE_VISIBILITY _Tp
+__libcpp_atomic_refcount_decrement(_Tp& __t) _NOEXCEPT
+{
+#if defined(_LIBCPP_HAS_BUILTIN_ATOMIC_SUPPORT) && !defined(_LIBCPP_HAS_NO_THREADS)
+return __atomic_add_fetch(&__t, -1, __ATOMIC_ACQ_REL);
+#else
+return __t -= 1;
+#endif
+}
+
 class _LIBCPP_EXCEPTION_ABI bad_weak_ptr
 : public std::exception
 {
@@ -3717,8 +3750,23 @@
 explicit __shared_count(long __refs = 0) _NOEXCEPT
 : __shared_owners_(__refs) {}
 
-void __add_shared() _NOEXCEPT;
-bool __release_shared() _NOEXCEPT;
+#ifdef _LIBCPP_BUILDING_MEMORY
+_LIBCPP_FUNC_VIS void __add_shared() _NOEXCEPT;
+_LIBCPP_FUNC_VIS bool __release_shared() _NOEXCEPT;
+#else
+_LIBCPP_INLINE_VISIBILITY
+void __add_shared() _NOEXCEPT {
+  __libcpp_atomic_refcount_increment(__shared_owners_);
+}
+_LIBCPP_INLINE_VISIBILITY
+bool __release_shared() _NOEXCEPT {
+  if (__libcpp_atomic_refcount_decrement(__shared_owners_) == -1) {
+__on_zero_shared();
+return true;
+  }
+  return false;
+}
+#endif
 _LIBCPP_INLINE_VISIBILITY
 long use_count() const _NOEXCEPT {
 return __libcpp_relaxed_load(&__shared_owners_) + 1;
@@ -3739,9 +3787,25 @@
 virtual ~__shared_weak_count();
 
 public:
-void __add_shared() _NOEXCEPT;
-void __add_weak() _NOEXCEPT;
-void __release_shared() _NOEXCEPT;
+#ifdef _LIBCPP_BUILDING_MEMORY
+_LIBCPP_FUNC_VIS void __add_shared() _NOEXCEPT;
+_LIBCPP_FUNC_VIS void __add_weak() _NOEXCEPT;
+_LIBCPP_FUNC_VIS void __release_shared() _NOEXCEPT;
+#else
+_LIBCPP_INLINE_VISIBILITY
+void __add_shared() _NOEXCEPT {
+  

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kevin Hu via Phabricator via cfe-commits]

hxy9243 updated this revision to Diff 84569.
hxy9243 added a comment.

Addresses comments from @mclow.lists.


Repository:
  rL LLVM

https://reviews.llvm.org/D24991

Files:
  libcxx/include/memory
  libcxx/src/memory.cpp

Index: libcxx/src/memory.cpp
===
--- libcxx/src/memory.cpp
+++ libcxx/src/memory.cpp
@@ -17,28 +17,6 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-namespace
-{
-
-// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
-// should be sufficient for thread safety.
-// See https://llvm.org/bugs/show_bug.cgi?id=22803
-template 
-inline T
-increment(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, 1, _AO_Relaxed);
-}
-
-template 
-inline T
-decrement(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, -1, _AO_Acq_Rel);
-}
-
-}  // namespace
-
 const allocator_arg_t allocator_arg = allocator_arg_t();
 
 bad_weak_ptr::~bad_weak_ptr() _NOEXCEPT {}
@@ -53,27 +31,27 @@
 {
 }
 
+__shared_weak_count::~__shared_weak_count()
+{
+}
+
 void
 __shared_count::__add_shared() _NOEXCEPT
 {
-increment(__shared_owners_);
+__libcpp_atomic_refcount_increment(__shared_owners_);
 }
 
 bool
 __shared_count::__release_shared() _NOEXCEPT
 {
-if (decrement(__shared_owners_) == -1)
+if (__libcpp_atomic_refcount_decrement(__shared_owners_) == -1)
 {
 __on_zero_shared();
 return true;
 }
 return false;
 }
 
-__shared_weak_count::~__shared_weak_count()
-{
-}
-
 void
 __shared_weak_count::__add_shared() _NOEXCEPT
 {
@@ -83,7 +61,7 @@
 void
 __shared_weak_count::__add_weak() _NOEXCEPT
 {
-increment(__shared_weak_owners_);
+__libcpp_atomic_refcount_increment(__shared_weak_owners_);
 }
 
 void
@@ -124,7 +102,7 @@
 //__libcpp_atomic_store(&__shared_weak_owners_, -1, _AO_Release);
 __on_zero_shared_weak();
 }
-else if (decrement(__shared_weak_owners_) == -1)
+else if (__libcpp_atomic_refcount_decrement(__shared_weak_owners_) == -1)
 __on_zero_shared_weak();
 }
 
Index: libcxx/include/memory
===
--- libcxx/include/memory
+++ libcxx/include/memory
@@ -3681,6 +3681,39 @@
 
 #endif // _LIBCPP_STD_VER > 14
 
+// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
+// should be sufficient for thread safety.
+// See https://llvm.org/bugs/show_bug.cgi?id=22803
+#if defined(__clang__) && __has_builtin(__atomic_add_fetch)  \
+   && defined(__ATOMIC_RELAXED)  \
+   && defined(__ATOMIC_ACQ_REL)
+#   define _LIBCPP_HAS_BUILTIN_ATOMIC_SUPPORT
+#elif !defined(__clang__) && defined(_GNUC_VER) && _GNUC_VER >= 407
+#   define _LIBCPP_HAS_BUILTIN_ATOMIC_SUPPORT
+#endif
+
+template 
+inline _LIBCPP_INLINE_VISIBILITY _Tp
+__libcpp_atomic_refcount_increment(_Tp& __t) _NOEXCEPT
+{
+#if defined(_LIBCPP_HAS_BUILTIN_ATOMIC_SUPPORT) && !defined(_LIBCPP_HAS_NO_THREADS)
+return __atomic_add_fetch(&__t, 1, __ATOMIC_RELAXED);
+#else
+return __t += 1;
+#endif
+}
+
+template 
+inline _LIBCPP_INLINE_VISIBILITY _Tp
+__libcpp_atomic_refcount_decrement(_Tp& __t) _NOEXCEPT
+{
+#if defined(_LIBCPP_HAS_BUILTIN_ATOMIC_SUPPORT) && !defined(_LIBCPP_HAS_NO_THREADS)
+return __atomic_add_fetch(&__t, -1, __ATOMIC_ACQ_REL);
+#else
+return __t -= 1;
+#endif
+}
+
 class _LIBCPP_EXCEPTION_ABI bad_weak_ptr
 : public std::exception
 {
@@ -3717,8 +3750,23 @@
 explicit __shared_count(long __refs = 0) _NOEXCEPT
 : __shared_owners_(__refs) {}
 
-void __add_shared() _NOEXCEPT;
-bool __release_shared() _NOEXCEPT;
+#ifdef _LIBCPP_BUILDING_MEMORY
+_LIBCPP_FUNC_VIS void __add_shared() _NOEXCEPT;
+_LIBCPP_FUNC_VIS bool __release_shared() _NOEXCEPT;
+#else
+_LIBCPP_INLINE_VISIBILITY
+void __add_shared() _NOEXCEPT {
+  __libcpp_atomic_refcount_increment(__shared_owners_);
+}
+_LIBCPP_INLINE_VISIBILITY
+bool __release_shared() _NOEXCEPT {
+  if (__libcpp_atomic_refcount_decrement(__shared_owners_) == -1) {
+__on_zero_shared();
+return true;
+  }
+  return false;
+}
+#endif
 _LIBCPP_INLINE_VISIBILITY
 long use_count() const _NOEXCEPT {
 return __libcpp_relaxed_load(&__shared_owners_) + 1;
@@ -3739,9 +3787,25 @@
 virtual ~__shared_weak_count();
 
 public:
-void __add_shared() _NOEXCEPT;
-void __add_weak() _NOEXCEPT;
-void __release_shared() _NOEXCEPT;
+#ifdef _LIBCPP_BUILDING_MEMORY
+_LIBCPP_FUNC_VIS void __add_shared() _NOEXCEPT;
+_LIBCPP_FUNC_VIS void __add_weak() _NOEXCEPT;
+_LIBCPP_FUNC_VIS void __release_shared() _NOEXCEPT;
+#else
+_LIBCPP_INLINE_VISIBILITY
+void __add_shared() _NOEXCEPT {
+  __shared_count::__add_shared();
+}
+_LIBCPP_INLINE_VISIBILITY
+void __add_weak() _NOEXCEPT {
+  __libcpp_atomic_refcount_increment(__shared_weak_owners_);
+}
+_LIBCPP_INLINE_VISIBILITY
+void 

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: libcxx/include/memory:3700
+
+template 
+inline T

`template `, please.

Otherwise when some client code does `#define T true` (yes, I've seen that!) 
this breaks.  `_Tp` is a reserved identifier, and if they use that, we can 
point at them and laugh.




Comment at: libcxx/include/memory:3702
+inline T
+__libcpp_atomic_refcount_increment(T& t) _NOEXCEPT
+{

The parameter name needs to be reserved as well. `__t`, please.



Comment at: libcxx/include/memory:3711
+
+template 
+inline T

Same comment as L3700



Comment at: libcxx/include/memory:3713
+inline T
+__libcpp_atomic_refcount_decrement(T& t) _NOEXCEPT
+{

Same comment as L3702


Repository:
  rL LLVM

https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kevin Hu via Phabricator via cfe-commits]

hxy9243 updated this revision to Diff 84565.
hxy9243 added a comment.

Addresses comments from @EricWF.

Thanks for reviewing, I know it takes a lot of energy. It helped me learn a lot.


Repository:
  rL LLVM

https://reviews.llvm.org/D24991

Files:
  libcxx/include/memory
  libcxx/src/memory.cpp

Index: libcxx/src/memory.cpp
===
--- libcxx/src/memory.cpp
+++ libcxx/src/memory.cpp
@@ -17,28 +17,6 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-namespace
-{
-
-// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
-// should be sufficient for thread safety.
-// See https://llvm.org/bugs/show_bug.cgi?id=22803
-template 
-inline T
-increment(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, 1, _AO_Relaxed);
-}
-
-template 
-inline T
-decrement(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, -1, _AO_Acq_Rel);
-}
-
-}  // namespace
-
 const allocator_arg_t allocator_arg = allocator_arg_t();
 
 bad_weak_ptr::~bad_weak_ptr() _NOEXCEPT {}
@@ -53,27 +31,27 @@
 {
 }
 
+__shared_weak_count::~__shared_weak_count()
+{
+}
+
 void
 __shared_count::__add_shared() _NOEXCEPT
 {
-increment(__shared_owners_);
+__libcpp_atomic_refcount_increment(__shared_owners_);
 }
 
 bool
 __shared_count::__release_shared() _NOEXCEPT
 {
-if (decrement(__shared_owners_) == -1)
+if (__libcpp_atomic_refcount_decrement(__shared_owners_) == -1)
 {
 __on_zero_shared();
 return true;
 }
 return false;
 }
 
-__shared_weak_count::~__shared_weak_count()
-{
-}
-
 void
 __shared_weak_count::__add_shared() _NOEXCEPT
 {
@@ -83,7 +61,7 @@
 void
 __shared_weak_count::__add_weak() _NOEXCEPT
 {
-increment(__shared_weak_owners_);
+__libcpp_atomic_refcount_increment(__shared_weak_owners_);
 }
 
 void
@@ -124,7 +102,7 @@
 //__libcpp_atomic_store(&__shared_weak_owners_, -1, _AO_Release);
 __on_zero_shared_weak();
 }
-else if (decrement(__shared_weak_owners_) == -1)
+else if (__libcpp_atomic_refcount_decrement(__shared_weak_owners_) == -1)
 __on_zero_shared_weak();
 }
 
Index: libcxx/include/memory
===
--- libcxx/include/memory
+++ libcxx/include/memory
@@ -3681,6 +3681,44 @@
 
 #endif // _LIBCPP_STD_VER > 14
 
+// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
+// should be sufficient for thread safety.
+// See https://llvm.org/bugs/show_bug.cgi?id=22803
+#if defined(__clang__) && __has_builtin(__atomic_load_n) \
+   && __has_builtin(__atomic_store_n)\
+   && __has_builtin(__atomic_add_fetch)  \
+   && __has_builtin(__atomic_compare_exchange_n) \
+   && defined(__ATOMIC_RELAXED)  \
+   && defined(__ATOMIC_CONSUME)  \
+   && defined(__ATOMIC_ACQUIRE)  \
+   && defined(__ATOMIC_RELEASE)  \
+   && defined(__ATOMIC_ACQ_REL)  \
+   && defined(__ATOMIC_SEQ_CST)
+#   define _LIBCPP_HAS_ATOMIC_BUILTINS
+#endif
+
+template 
+inline T
+__libcpp_atomic_refcount_increment(T& t) _NOEXCEPT
+{
+#if defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
+return __atomic_add_fetch(, 1, __ATOMIC_RELAXED);
+#else
+return t += 1;
+#endif
+}
+
+template 
+inline T
+__libcpp_atomic_refcount_decrement(T& t) _NOEXCEPT
+{
+#if defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
+return __atomic_add_fetch(, -1, __ATOMIC_ACQ_REL);
+#else
+return t -= 1;
+#endif
+}
+
 class _LIBCPP_EXCEPTION_ABI bad_weak_ptr
 : public std::exception
 {
@@ -3717,8 +3755,23 @@
 explicit __shared_count(long __refs = 0) _NOEXCEPT
 : __shared_owners_(__refs) {}
 
-void __add_shared() _NOEXCEPT;
-bool __release_shared() _NOEXCEPT;
+#ifdef _LIBCPP_BUILDING_MEMORY
+void _LIBCPP_FUNC_VIS __add_shared() _NOEXCEPT;
+bool _LIBCPP_FUNC_VIS __release_shared() _NOEXCEPT;
+#else
+_LIBCPP_INLINE_VISIBILITY
+void __add_shared() _NOEXCEPT {
+  __libcpp_atomic_refcount_increment(__shared_owners_);
+}
+_LIBCPP_INLINE_VISIBILITY
+bool __release_shared() _NOEXCEPT {
+  if (__libcpp_atomic_refcount_decrement(__shared_owners_) == -1) {
+__on_zero_shared();
+return true;
+  }
+  return false;
+}
+#endif
 _LIBCPP_INLINE_VISIBILITY
 long use_count() const _NOEXCEPT {
 return __libcpp_relaxed_load(&__shared_owners_) + 1;
@@ -3739,9 +3792,25 @@
 virtual ~__shared_weak_count();
 
 public:
-void __add_shared() _NOEXCEPT;
-void __add_weak() _NOEXCEPT;
-void __release_shared() _NOEXCEPT;
+#ifdef _LIBCPP_BUILDING_MEMORY
+void _LIBCPP_FUNC_VIS __add_shared() _NOEXCEPT;
+void _LIBCPP_FUNC_VIS __add_weak() 

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kevin Hu via Phabricator via cfe-commits]

hxy9243 updated this revision to Diff 84004.
hxy9243 added a comment.

Minor changes on function names. Rename __libcpp_atomic_* to 
__libcpp_atomic_refcount_*.


Repository:
  rL LLVM

https://reviews.llvm.org/D24991

Files:
  libcxx/include/memory
  libcxx/src/memory.cpp

Index: libcxx/src/memory.cpp
===
--- libcxx/src/memory.cpp
+++ libcxx/src/memory.cpp
@@ -17,28 +17,6 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-namespace
-{
-
-// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
-// should be sufficient for thread safety.
-// See https://llvm.org/bugs/show_bug.cgi?id=22803
-template 
-inline T
-increment(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, 1, _AO_Relaxed);
-}
-
-template 
-inline T
-decrement(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, -1, _AO_Acq_Rel);
-}
-
-}  // namespace
-
 const allocator_arg_t allocator_arg = allocator_arg_t();
 
 bad_weak_ptr::~bad_weak_ptr() _NOEXCEPT {}
@@ -53,27 +31,27 @@
 {
 }
 
+__shared_weak_count::~__shared_weak_count()
+{
+}
+
 void
 __shared_count::__add_shared() _NOEXCEPT
 {
-increment(__shared_owners_);
+__libcpp_atomic_refcount_increment(__shared_owners_);
 }
 
 bool
 __shared_count::__release_shared() _NOEXCEPT
 {
-if (decrement(__shared_owners_) == -1)
+if (__libcpp_atomic_refcount_decrement(__shared_owners_) == -1)
 {
 __on_zero_shared();
 return true;
 }
 return false;
 }
 
-__shared_weak_count::~__shared_weak_count()
-{
-}
-
 void
 __shared_weak_count::__add_shared() _NOEXCEPT
 {
@@ -83,7 +61,7 @@
 void
 __shared_weak_count::__add_weak() _NOEXCEPT
 {
-increment(__shared_weak_owners_);
+__libcpp_atomic_refcount_increment(__shared_weak_owners_);
 }
 
 void
@@ -124,7 +102,7 @@
 //__libcpp_atomic_store(&__shared_weak_owners_, -1, _AO_Release);
 __on_zero_shared_weak();
 }
-else if (decrement(__shared_weak_owners_) == -1)
+else if (__libcpp_atomic_refcount_decrement(__shared_weak_owners_) == -1)
 __on_zero_shared_weak();
 }
 
Index: libcxx/include/memory
===
--- libcxx/include/memory
+++ libcxx/include/memory
@@ -3681,6 +3681,44 @@
 
 #endif // _LIBCPP_STD_VER > 14
 
+// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
+// should be sufficient for thread safety.
+// See https://llvm.org/bugs/show_bug.cgi?id=22803
+#if defined(__clang__) && __has_builtin(__atomic_load_n) \
+   && __has_builtin(__atomic_store_n)\
+   && __has_builtin(__atomic_add_fetch)  \
+   && __has_builtin(__atomic_compare_exchange_n) \
+   && defined(__ATOMIC_RELAXED)  \
+   && defined(__ATOMIC_CONSUME)  \
+   && defined(__ATOMIC_ACQUIRE)  \
+   && defined(__ATOMIC_RELEASE)  \
+   && defined(__ATOMIC_ACQ_REL)  \
+   && defined(__ATOMIC_SEQ_CST)
+#   define _LIBCPP_HAS_ATOMIC_BUILTINS
+#endif
+
+template 
+inline T
+__libcpp_atomic_refcount_increment(T& t) _NOEXCEPT
+{
+#if defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
+return __atomic_add_fetch(, 1, __ATOMIC_RELAXED);
+#else
+return t += 1;
+#endif
+}
+
+template 
+inline T
+__libcpp_atomic_refcount_decrement(T& t) _NOEXCEPT
+{
+#if defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
+return __atomic_add_fetch(, -1, __ATOMIC_ACQ_REL);
+#else
+return t -= 1;
+#endif
+}
+
 class _LIBCPP_EXCEPTION_ABI bad_weak_ptr
 : public std::exception
 {
@@ -3717,8 +3755,23 @@
 explicit __shared_count(long __refs = 0) _NOEXCEPT
 : __shared_owners_(__refs) {}
 
-void __add_shared() _NOEXCEPT;
-bool __release_shared() _NOEXCEPT;
+#ifdef _LIBCPP_BUILDING_MEMORY
+void _LIBCPP_FUNC_VIS __add_shared() _NOEXCEPT;
+bool _LIBCPP_FUNC_VIS __release_shared() _NOEXCEPT;
+#else
+_LIBCPP_INLINE_VISIBILITY
+void __add_shared() _NOEXCEPT {
+  __libcpp_atomic_refcount_increment(__shared_owners_);
+}
+_LIBCPP_INLINE_VISIBILITY
+bool __release_shared() _NOEXCEPT {
+  if (__libcpp_atomic_refcount_decrement(__shared_owners_) == -1) {
+__on_zero_shared();
+return true;
+  }
+  return false;
+}
+#endif
 _LIBCPP_INLINE_VISIBILITY
 long use_count() const _NOEXCEPT {
 return __libcpp_relaxed_load(&__shared_owners_) + 1;
@@ -3739,9 +3792,25 @@
 virtual ~__shared_weak_count();
 
 public:
-void __add_shared() _NOEXCEPT;
-void __add_weak() _NOEXCEPT;
-void __release_shared() _NOEXCEPT;
+#ifdef _LIBCPP_BUILDING_MEMORY
+void _LIBCPP_FUNC_VIS __add_shared() _NOEXCEPT;
+void _LIBCPP_FUNC_VIS __add_weak() _NOEXCEPT;
+void 

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kuba (Brecka) Mracek via Phabricator via cfe-commits]

kubabrecka added inline comments.



Comment at: libcxx/include/memory:3702
+inline T
+__libcpp_atomic_increment(T& t) _NOEXCEPT
+{

I don't think this should be named `__libcpp_atomic_increment`, because it uses 
relaxed ordering and thus it's not a generic increment (same goes for 
decrement).  Could we rename this to `__libcpp_atomic_refcount_increment` or 
something similar?  That would suggest why we're using acq+rel on one side and 
relaxed on other side.  Using these functions for non-refcount purposes will be 
wrong and the current names (`__libcpp_atomic_increment`) suggest that they're 
doing generic atomic operations.


Repository:
  rL LLVM

https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kevin Hu via Phabricator via cfe-commits]

hxy9243 marked an inline comment as done.
hxy9243 added a comment.

Addressed previous issues in the comments. The patch still shows consistent 
perf uplift in proprietary benchmark on shared_ptr.

@EricWF @sebpop @hiraditya Any thoughts?


Repository:
  rL LLVM

https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kevin Hu via Phabricator via cfe-commits]

hxy9243 updated this revision to Diff 82962.
hxy9243 marked 4 inline comments as done.
hxy9243 added a comment.

Minor fix, remove redundant inlines.


Repository:
  rL LLVM

https://reviews.llvm.org/D24991

Files:
  libcxx/include/memory
  libcxx/src/memory.cpp

Index: libcxx/src/memory.cpp
===
--- libcxx/src/memory.cpp
+++ libcxx/src/memory.cpp
@@ -17,28 +17,6 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-namespace
-{
-
-// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
-// should be sufficient for thread safety.
-// See https://llvm.org/bugs/show_bug.cgi?id=22803
-template 
-inline T
-increment(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, 1, _AO_Relaxed);
-}
-
-template 
-inline T
-decrement(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, -1, _AO_Acq_Rel);
-}
-
-}  // namespace
-
 const allocator_arg_t allocator_arg = allocator_arg_t();
 
 bad_weak_ptr::~bad_weak_ptr() _NOEXCEPT {}
@@ -53,27 +31,27 @@
 {
 }
 
+__shared_weak_count::~__shared_weak_count()
+{
+}
+
 void
 __shared_count::__add_shared() _NOEXCEPT
 {
-increment(__shared_owners_);
+__libcpp_atomic_increment(__shared_owners_);
 }
 
 bool
 __shared_count::__release_shared() _NOEXCEPT
 {
-if (decrement(__shared_owners_) == -1)
+if (__libcpp_atomic_decrement(__shared_owners_) == -1)
 {
 __on_zero_shared();
 return true;
 }
 return false;
 }
 
-__shared_weak_count::~__shared_weak_count()
-{
-}
-
 void
 __shared_weak_count::__add_shared() _NOEXCEPT
 {
@@ -83,7 +61,7 @@
 void
 __shared_weak_count::__add_weak() _NOEXCEPT
 {
-increment(__shared_weak_owners_);
+__libcpp_atomic_increment(__shared_weak_owners_);
 }
 
 void
@@ -124,7 +102,7 @@
 //__libcpp_atomic_store(&__shared_weak_owners_, -1, _AO_Release);
 __on_zero_shared_weak();
 }
-else if (decrement(__shared_weak_owners_) == -1)
+else if (__libcpp_atomic_decrement(__shared_weak_owners_) == -1)
 __on_zero_shared_weak();
 }
 
Index: libcxx/include/memory
===
--- libcxx/include/memory
+++ libcxx/include/memory
@@ -3681,6 +3681,44 @@
 
 #endif // _LIBCPP_STD_VER > 14
 
+// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
+// should be sufficient for thread safety.
+// See https://llvm.org/bugs/show_bug.cgi?id=22803
+#if defined(__clang__) && __has_builtin(__atomic_load_n) \
+   && __has_builtin(__atomic_store_n)\
+   && __has_builtin(__atomic_add_fetch)  \
+   && __has_builtin(__atomic_compare_exchange_n) \
+   && defined(__ATOMIC_RELAXED)  \
+   && defined(__ATOMIC_CONSUME)  \
+   && defined(__ATOMIC_ACQUIRE)  \
+   && defined(__ATOMIC_RELEASE)  \
+   && defined(__ATOMIC_ACQ_REL)  \
+   && defined(__ATOMIC_SEQ_CST)
+#   define _LIBCPP_HAS_ATOMIC_BUILTINS
+#endif
+
+template 
+inline T
+__libcpp_atomic_increment(T& t) _NOEXCEPT
+{
+#if defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
+return __atomic_add_fetch(, 1, __ATOMIC_RELAXED);
+#else
+return t += 1;
+#endif
+}
+
+template 
+inline T
+__libcpp_atomic_decrement(T& t) _NOEXCEPT
+{
+#if defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
+return __atomic_add_fetch(, -1, __ATOMIC_ACQ_REL);
+#else
+return t -= 1;
+#endif
+}
+
 class _LIBCPP_EXCEPTION_ABI bad_weak_ptr
 : public std::exception
 {
@@ -3717,8 +3755,23 @@
 explicit __shared_count(long __refs = 0) _NOEXCEPT
 : __shared_owners_(__refs) {}
 
-void __add_shared() _NOEXCEPT;
-bool __release_shared() _NOEXCEPT;
+#ifdef _LIBCPP_BUILDING_MEMORY
+void _LIBCPP_FUNC_VIS __add_shared() _NOEXCEPT;
+bool _LIBCPP_FUNC_VIS __release_shared() _NOEXCEPT;
+#else
+_LIBCPP_INLINE_VISIBILITY
+void __add_shared() _NOEXCEPT {
+  __libcpp_atomic_increment(__shared_owners_);
+}
+_LIBCPP_INLINE_VISIBILITY
+bool __release_shared() _NOEXCEPT {
+  if (__libcpp_atomic_decrement(__shared_owners_) == -1) {
+__on_zero_shared();
+return true;
+  }
+  return false;
+}
+#endif
 _LIBCPP_INLINE_VISIBILITY
 long use_count() const _NOEXCEPT {
 return __libcpp_relaxed_load(&__shared_owners_) + 1;
@@ -3739,9 +3792,25 @@
 virtual ~__shared_weak_count();
 
 public:
-void __add_shared() _NOEXCEPT;
-void __add_weak() _NOEXCEPT;
-void __release_shared() _NOEXCEPT;
+#ifdef _LIBCPP_BUILDING_MEMORY
+void _LIBCPP_FUNC_VIS __add_shared() _NOEXCEPT;
+void _LIBCPP_FUNC_VIS __add_weak() _NOEXCEPT;
+void _LIBCPP_FUNC_VIS __release_shared() _NOEXCEPT;
+#else
+_LIBCPP_INLINE_VISIBILITY
+

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kevin Hu via Phabricator via cfe-commits]

hxy9243 set the repository for this revision to rL LLVM.
hxy9243 updated this revision to Diff 82958.
hxy9243 added a comment.

Move the header back in its place, and only copy over necessary parts. Now call 
__atomic_add_fetch from inside the functions.


Repository:
  rL LLVM

https://reviews.llvm.org/D24991

Files:
  libcxx/include/memory
  libcxx/src/memory.cpp

Index: libcxx/src/memory.cpp
===
--- libcxx/src/memory.cpp
+++ libcxx/src/memory.cpp
@@ -17,28 +17,6 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-namespace
-{
-
-// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
-// should be sufficient for thread safety.
-// See https://llvm.org/bugs/show_bug.cgi?id=22803
-template 
-inline T
-increment(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, 1, _AO_Relaxed);
-}
-
-template 
-inline T
-decrement(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, -1, _AO_Acq_Rel);
-}
-
-}  // namespace
-
 const allocator_arg_t allocator_arg = allocator_arg_t();
 
 bad_weak_ptr::~bad_weak_ptr() _NOEXCEPT {}
@@ -53,27 +31,27 @@
 {
 }
 
+__shared_weak_count::~__shared_weak_count()
+{
+}
+
 void
 __shared_count::__add_shared() _NOEXCEPT
 {
-increment(__shared_owners_);
+__libcpp_atomic_increment(__shared_owners_);
 }
 
 bool
 __shared_count::__release_shared() _NOEXCEPT
 {
-if (decrement(__shared_owners_) == -1)
+if (__libcpp_atomic_decrement(__shared_owners_) == -1)
 {
 __on_zero_shared();
 return true;
 }
 return false;
 }
 
-__shared_weak_count::~__shared_weak_count()
-{
-}
-
 void
 __shared_weak_count::__add_shared() _NOEXCEPT
 {
@@ -83,7 +61,7 @@
 void
 __shared_weak_count::__add_weak() _NOEXCEPT
 {
-increment(__shared_weak_owners_);
+__libcpp_atomic_increment(__shared_weak_owners_);
 }
 
 void
@@ -124,7 +102,7 @@
 //__libcpp_atomic_store(&__shared_weak_owners_, -1, _AO_Release);
 __on_zero_shared_weak();
 }
-else if (decrement(__shared_weak_owners_) == -1)
+else if (__libcpp_atomic_decrement(__shared_weak_owners_) == -1)
 __on_zero_shared_weak();
 }
 
Index: libcxx/include/memory
===
--- libcxx/include/memory
+++ libcxx/include/memory
@@ -3681,6 +3681,44 @@
 
 #endif // _LIBCPP_STD_VER > 14
 
+// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
+// should be sufficient for thread safety.
+// See https://llvm.org/bugs/show_bug.cgi?id=22803
+#if defined(__clang__) && __has_builtin(__atomic_load_n) \
+   && __has_builtin(__atomic_store_n)\
+   && __has_builtin(__atomic_add_fetch)  \
+   && __has_builtin(__atomic_compare_exchange_n) \
+   && defined(__ATOMIC_RELAXED)  \
+   && defined(__ATOMIC_CONSUME)  \
+   && defined(__ATOMIC_ACQUIRE)  \
+   && defined(__ATOMIC_RELEASE)  \
+   && defined(__ATOMIC_ACQ_REL)  \
+   && defined(__ATOMIC_SEQ_CST)
+#   define _LIBCPP_HAS_ATOMIC_BUILTINS
+#endif
+
+template 
+inline T
+__libcpp_atomic_increment(T& t) _NOEXCEPT
+{
+#if defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
+return __atomic_add_fetch(, 1, __ATOMIC_RELAXED);
+#else
+return t += 1;
+#endif
+}
+
+template 
+inline T
+__libcpp_atomic_decrement(T& t) _NOEXCEPT
+{
+#if defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
+return __atomic_add_fetch(, -1, __ATOMIC_ACQ_REL);
+#else
+return t -= 1;
+#endif
+}
+
 class _LIBCPP_EXCEPTION_ABI bad_weak_ptr
 : public std::exception
 {
@@ -3717,8 +3755,23 @@
 explicit __shared_count(long __refs = 0) _NOEXCEPT
 : __shared_owners_(__refs) {}
 
-void __add_shared() _NOEXCEPT;
-bool __release_shared() _NOEXCEPT;
+#ifdef _LIBCPP_BUILDING_MEMORY
+void _LIBCPP_FUNC_VIS __add_shared() _NOEXCEPT;
+bool _LIBCPP_FUNC_VIS __release_shared() _NOEXCEPT;
+#else
+inline _LIBCPP_INLINE_VISIBILITY
+void __add_shared() _NOEXCEPT {
+  __libcpp_atomic_increment(__shared_owners_);
+}
+inline _LIBCPP_INLINE_VISIBILITY
+bool __release_shared() _NOEXCEPT {
+  if (__libcpp_atomic_decrement(__shared_owners_) == -1) {
+__on_zero_shared();
+return true;
+  }
+  return false;
+}
+#endif
 _LIBCPP_INLINE_VISIBILITY
 long use_count() const _NOEXCEPT {
 return __libcpp_relaxed_load(&__shared_owners_) + 1;
@@ -3739,9 +3792,25 @@
 virtual ~__shared_weak_count();
 
 public:
-void __add_shared() _NOEXCEPT;
-void __add_weak() _NOEXCEPT;
-void __release_shared() _NOEXCEPT;
+#ifdef _LIBCPP_BUILDING_MEMORY
+void _LIBCPP_FUNC_VIS __add_shared() _NOEXCEPT;
+void _LIBCPP_FUNC_VIS 

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF requested changes to this revision.
EricWF added a reviewer: EricWF.
EricWF added a comment.
This revision now requires changes to proceed.

Added a bunch of inline comments.

The biggest requested change is removing the `__atomic_support` header. We only 
need one atomic call within the headers. It's overkill to add a new header.




Comment at: libcxx/include/__atomic_support:1
+//===--===
+//

I would greatly prefer if this patch didn't add another header, and simply 
defined `__libcpp_atomic_increment` and `__libcpp_atomic_decrement` in place of 
`__atomic_inc_dec::increment`/`__atomic_inc_dec::decrement`.



Comment at: libcxx/include/memory:3802
+{
+  return __libcpp_atomic_add(, 1, _AO_Relaxed);
+}

sebpop wrote:
> EricWF wrote:
> > Why add `increment` and `decrement` at all? Just manually inline 
> > `__libcpp_atomic_add` at the callsites.
> I like the idea to manually inline the inc and dec functions.
> What should we do with the NOTE: above?
> 
> // NOTE: Relaxed and acq/rel atomics (for increment and decrement 
> respectively)
> // should be sufficient for thread safety.
> // See https://llvm.org/bugs/show_bug.cgi?id=22803
> 
> should we just go ahead and remove the note, or you want to have it where 
> inc/dec are called?  (about a dozen places.) 
Neremind about the manually inlining bit.

Please remove the `__atomic_inc_dec` namespace and rename `increment` to 
`__libcpp_atomic_increment` and `decrement` to `__libcpp_atomic_decrement`.

Please also remove the `__atomic_support` header and instead simply call 
`__atomic_add_fetch` from inside the functions.



Comment at: libcxx/include/memory:3911
+#ifdef _LIBCPP_BUILDING_MEMORY
 void __add_shared() _NOEXCEPT;
 bool __release_shared() _NOEXCEPT;

Please apply `_LIBCPP_FUNC_VIS` to both of these methods.



Comment at: libcxx/include/memory:3914
+#else
+inline _LIBCPP_INLINE_VISIBILITY
+void __add_shared() _NOEXCEPT {

the `inline` in redundant if you define the function inside the class.



Comment at: libcxx/include/memory:3948
+#ifdef _LIBCPP_BUILDING_MEMORY
 void __add_shared() _NOEXCEPT;
 void __add_weak() _NOEXCEPT;

Please add `_LIBCPP_FUNC_VIS` to the three methods.



Comment at: libcxx/include/memory:3952
+#else
+inline _LIBCPP_INLINE_VISIBILITY
+void __add_shared() _NOEXCEPT {

`inline` is redundant here.


https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Sebastian Pop via Phabricator via cfe-commits]

sebpop added a comment.

@mclow.lists could you please have a last look at this patch: the change is for 
a performance improvement (20% uplift on a proprietary benchmark), and all the 
issues mentioned in the review have been addressed.
The existing synthetic benchmark shows an overall improvement:

  master:
  Benchmark  Time   CPU Iterations
  
  BM_SharedPtrCreateDestroy 54 ns 54 ns   12388755
  BM_SharedPtrIncDecRef 37 ns 37 ns   19021739
  BM_WeakPtrIncDecRef   38 ns 38 ns   18421053
   
  master + patch:
  Benchmark  Time   CPU Iterations
  
  BM_SharedPtrCreateDestroy 44 ns 44 ns   14730639
  BM_SharedPtrIncDecRef 18 ns 18 ns   3889
  BM_WeakPtrIncDecRef   30 ns 30 ns   23648649


https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kevin Hu via cfe-commits]

hxy9243 added a comment.

Ping. @mclow.lists, @EricWF, any ideas on this patch?
Thanks very much!


https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Sebastian Pop via cfe-commits]

sebpop added a comment.

@mclow.lists, @EricWF, ok to commit the patch?

Thanks,
Sebastian


https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kuba Brecka via cfe-commits]

kubabrecka added a comment.

This passes TSan tests on Darwin. LGTM.


https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kevin Hu via cfe-commits]

hxy9243 added a comment.

In https://reviews.llvm.org/D24991#586248, @kubabrecka wrote:

> In https://reviews.llvm.org/D24991#586219, @sebpop wrote:
>
> > I just ran ninja check-all with and without this patch and there are no 
> > regressions in compiler-rt on an x86_64-linux machine.
>
>
> The TSan interceptors (and testcases) are Darwin-only at this point.  I'll 
> run the tests on my machine.


Any updates on the testing? Thanks very much!


https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kuba Brecka via cfe-commits]

kubabrecka added a comment.

In https://reviews.llvm.org/D24991#586219, @sebpop wrote:

> I just ran ninja check-all with and without this patch and there are no 
> regressions in compiler-rt on an x86_64-linux machine.


The TSan interceptors (and testcases) are Darwin-only at this point.  I'll run 
the tests on my machine.


https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Sebastian Pop via cfe-commits]

sebpop added a comment.

In https://reviews.llvm.org/D24991#583877, @kubabrecka wrote:

> Just a question:  TSan intercepts on the dylib functions, namely 
> `__release_shared`, to track the atomic accesses.  Can you make sure this 
> doesn't break?  There's a few testcases for this in compiler-rt.


I just ran ninja check-all with and without this patch and there are no 
regressions in compiler-rt on an x86_64-linux machine.


https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kuba Brecka via cfe-commits]

kubabrecka added a comment.

Just a question:  TSan intercepts on the dylib functions, namely 
`__release_shared`, to track the atomic accesses.  Can you make sure this 
doesn't break?  There's a few testcases for this in compiler-rt.


https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Sebastian Pop via cfe-commits]

sebpop added a comment.

Ping: Eric, Marshall, could you please approve or comment on this patch?
Thanks!


https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kevin Hu via cfe-commits]

hxy9243 accepted this revision.
hxy9243 added a comment.
This revision is now accepted and ready to land.

Looks good to me.
Notice that the performance gain can only be observed when compiled with the 
updated C++ header files.


https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Sebastian Pop via cfe-commits]

sebpop marked 2 inline comments as done.
sebpop added inline comments.



Comment at: libcxx/include/memory:3802
+{
+  return __libcpp_atomic_add(, 1, _AO_Relaxed);
+}

EricWF wrote:
> Why add `increment` and `decrement` at all? Just manually inline 
> `__libcpp_atomic_add` at the callsites.
I like the idea to manually inline the inc and dec functions.
What should we do with the NOTE: above?

// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
// should be sufficient for thread safety.
// See https://llvm.org/bugs/show_bug.cgi?id=22803

should we just go ahead and remove the note, or you want to have it where 
inc/dec are called?  (about a dozen places.) 


https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Sebastian Pop via cfe-commits]

sebpop removed rL LLVM as the repository for this revision.
sebpop updated this revision to Diff 75908.
sebpop added a comment.

The patch also implements the idea that Marshall proposed in:
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20161010/173780.html

> I have an idea; it involves a macro that is sometimes "inline" and
>  sometimes not, and changes when you're building the library vs. when you're
>  just including the headers.

Tested on x86_64-linux.
The symbols for the functions that are now inlined still appear in the 
libc++.so.

Ok to commit?


https://reviews.llvm.org/D24991

Files:
  libcxx/include/__atomic_support
  libcxx/include/memory
  libcxx/src/include/atomic_support.h
  libcxx/src/memory.cpp
  libcxx/src/mutex.cpp

Index: libcxx/src/mutex.cpp
===
--- libcxx/src/mutex.cpp
+++ libcxx/src/mutex.cpp
@@ -12,7 +12,7 @@
 #include "limits"
 #include "system_error"
 #include "cassert"
-#include "include/atomic_support.h"
+#include "__atomic_support"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 #ifndef _LIBCPP_HAS_NO_THREADS
Index: libcxx/src/memory.cpp
===
--- libcxx/src/memory.cpp
+++ libcxx/src/memory.cpp
@@ -13,32 +13,10 @@
 #include "mutex"
 #include "thread"
 #endif
-#include "include/atomic_support.h"
+#include "__atomic_support"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-namespace
-{
-
-// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
-// should be sufficient for thread safety.
-// See https://llvm.org/bugs/show_bug.cgi?id=22803
-template 
-inline T
-increment(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, 1, _AO_Relaxed);
-}
-
-template 
-inline T
-decrement(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, -1, _AO_Acq_Rel);
-}
-
-}  // namespace
-
 const allocator_arg_t allocator_arg = allocator_arg_t();
 
 bad_weak_ptr::~bad_weak_ptr() _NOEXCEPT {}
@@ -53,27 +31,27 @@
 {
 }
 
+__shared_weak_count::~__shared_weak_count()
+{
+}
+
 void
 __shared_count::__add_shared() _NOEXCEPT
 {
-increment(__shared_owners_);
+__atomic_inc_dec::increment(__shared_owners_);
 }
 
 bool
 __shared_count::__release_shared() _NOEXCEPT
 {
-if (decrement(__shared_owners_) == -1)
+if (__atomic_inc_dec::decrement(__shared_owners_) == -1)
 {
 __on_zero_shared();
 return true;
 }
 return false;
 }
 
-__shared_weak_count::~__shared_weak_count()
-{
-}
-
 void
 __shared_weak_count::__add_shared() _NOEXCEPT
 {
@@ -83,7 +61,7 @@
 void
 __shared_weak_count::__add_weak() _NOEXCEPT
 {
-increment(__shared_weak_owners_);
+__atomic_inc_dec::increment(__shared_weak_owners_);
 }
 
 void
@@ -124,7 +102,7 @@
 //__libcpp_atomic_store(&__shared_weak_owners_, -1, _AO_Release);
 __on_zero_shared_weak();
 }
-else if (decrement(__shared_weak_owners_) == -1)
+else if (__atomic_inc_dec::decrement(__shared_weak_owners_) == -1)
 __on_zero_shared_weak();
 }
 
Index: libcxx/src/include/atomic_support.h
===
--- libcxx/src/include/atomic_support.h
+++ /dev/null
@@ -1,158 +0,0 @@
-//===--===
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is dual licensed under the MIT and the University of Illinois Open
-// Source Licenses. See LICENSE.TXT for details.
-//
-//===--===
-
-#ifndef ATOMIC_SUPPORT_H
-#define ATOMIC_SUPPORT_H
-
-#include "__config"
-#include "memory" // for __libcpp_relaxed_load
-
-#if defined(__clang__) && __has_builtin(__atomic_load_n) \
-   && __has_builtin(__atomic_store_n)\
-   && __has_builtin(__atomic_add_fetch)  \
-   && __has_builtin(__atomic_compare_exchange_n) \
-   && defined(__ATOMIC_RELAXED)  \
-   && defined(__ATOMIC_CONSUME)  \
-   && defined(__ATOMIC_ACQUIRE)  \
-   && defined(__ATOMIC_RELEASE)  \
-   && defined(__ATOMIC_ACQ_REL)  \
-   && defined(__ATOMIC_SEQ_CST)
-#   define _LIBCPP_HAS_ATOMIC_BUILTINS
-#elif !defined(__clang__) && defined(_GNUC_VER) && _GNUC_VER >= 407
-#   define _LIBCPP_HAS_ATOMIC_BUILTINS
-#endif
-
-#if !defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
-# if defined(_MSC_VER) && !defined(__clang__)
-_LIBCPP_WARNING("Building libc++ without __atomic builtins is unsupported")
-# else
-#   warning Building libc++ without __atomic builtins is unsupported
-# endif
-#endif
-
-_LIBCPP_BEGIN_NAMESPACE_STD
-
-namespace {
-
-#if defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
-
-enum 

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Sebastian Pop via cfe-commits]

sebpop added a comment.

In https://reviews.llvm.org/D24991#571056, @hiraditya wrote:

> Marshall suggests using macro as we discussed offline. For some reason the 
> reply does not appear here: 
> http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20161010/173780.html


Ping.


Repository:
  rL LLVM

https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Aditya Kumar via cfe-commits]

hiraditya added a comment.

Marshall suggests using macro as we discussed offline. For some reason the 
reply does not appear here: 
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20161010/173780.html


Repository:
  rL LLVM

https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Marshall Clow via cfe-commits]

On Thu, Oct 13, 2016 at 11:48 AM, Sebastian Pop  wrote:

> sebpop added a comment.
>
> In https://reviews.llvm.org/D24991#565861, @EricWF wrote:
>
> > In https://reviews.llvm.org/D24991#565715, @mclow.lists wrote:
> >
> > > How does this play with existing binaries?  Applications that expect
> these functions to exist in the dylib?
> >
> >
> > This patch is majorly ABI breaking, although we could probably find a
> formulation that wasn't.
>
>
> Eric, Marshall,
> any suggestions on how to fix the backwards compatibility issue?
>
>
The routine *has* to be in the dylib.
That being said, that doesn't mean that the version in the dylib must be
called.

I have an idea; it involves a macro that is sometimes "inline" and
sometimes not, and changes when you're building the library vs. when you're
just including the headers.

I'll play with that and put something up here.

-- Marshall
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Sebastian Pop via cfe-commits]

sebpop added a comment.

In https://reviews.llvm.org/D24991#565861, @EricWF wrote:

> In https://reviews.llvm.org/D24991#565715, @mclow.lists wrote:
>
> > How does this play with existing binaries?  Applications that expect these 
> > functions to exist in the dylib?
>
>
> This patch is majorly ABI breaking, although we could probably find a 
> formulation that wasn't.


Eric, Marshall,
any suggestions on how to fix the backwards compatibility issue?

Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kevin Hu via cfe-commits]

hxy9243 added a comment.

Thanks for pointing out. It's true that it may cause ABI breakage. It would be 
nice to keep compatibility while getting the performance benefits from inlining.

I've tested the patch with google-benchmark/util_smartptr_libcxx shipped with 
libcxx on x86_64 server, and attached the results as following:

  BASE libcxx r283113:
  $   taskset -c 23 ./util_smartptr.libcxx.out
  Run on (24 X 1200 MHz CPU s)
  2016-10-12 13:52:03
  ***WARNING*** CPU scaling is enabled, the benchmark real time measurements 
may be noisy and will incur extra overhead.
  Benchmark  Time   CPU Iterations
  
  BM_SharedPtrCreateDestroy 54 ns 54 ns   12388755
  BM_SharedPtrIncDecRef 37 ns 37 ns   19021739
  BM_WeakPtrIncDecRef   38 ns 38 ns   18421053
   
  
  
  libcxx with patch:
  $   taskset -c 23 ./util_smartptr.libcxx.out
  Run on (24 X 1200 MHz CPU s)
  2016-10-12 13:48:38
  ***WARNING*** CPU scaling is enabled, the benchmark real time measurements 
may be noisy and will incur extra overhead.
  Benchmark  Time   CPU Iterations
  
  BM_SharedPtrCreateDestroy 44 ns 44 ns   14730639
  BM_SharedPtrIncDecRef 18 ns 18 ns   3889
  BM_WeakPtrIncDecRef   30 ns 30 ns   23648649


Repository:
  rL LLVM

https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Eric Fiselier via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D24991#566140, @sebpop wrote:

> In https://reviews.llvm.org/D24991#565861, @EricWF wrote:
>
> > Please provide benchmark tests which demonstrate that these benefits are 
> > concrete and not just "potential".  Moving methods out of the dylib is no 
> > easy task so I would like hard evidence that it's worth while.
>
>
> With this patch we have seen the score of a proprietary benchmark going up by 
> 20%, matching the performance we see with LLVM + libstdc++. 
>  We will provide a testcase that shows the performance uplift.


20% sounds amazing! Thanks for working on this.


Repository:
  rL LLVM

https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Sebastian Pop via cfe-commits]

sebpop added a comment.

In https://reviews.llvm.org/D24991#565861, @EricWF wrote:

> Please provide benchmark tests which demonstrate that these benefits are 
> concrete and not just "potential".  Moving methods out of the dylib is no 
> easy task so I would like hard evidence that it's worth while.


With this patch we have seen the score of a proprietary benchmark going up by 
20%, matching the performance we see with LLVM + libstdc++. 
We will provide a testcase that shows the performance uplift.


Repository:
  rL LLVM

https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Eric Fiselier via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D24991#565715, @mclow.lists wrote:

> How does this play with existing binaries?  Applications that expect these 
> functions to exist in the dylib?


This patch is majorly ABI breaking, although we could probably find a 
formulation that wasn't.

@hxy9243 wrote:

>   which gives potential optimization opportunities and performance benefits.


Please provide benchmark tests which demonstrate that these benefits are 
concrete and not just "potential".  Moving methods out of the dylib is no easy 
task so I would like hard evidence that it's worth while.




Comment at: libcxx/include/memory:3793
 
+namespace
+{

Anonymous namespaces are a C++11 feature and this is C++03 code. 



Comment at: libcxx/include/memory:3798
+// See https://llvm.org/bugs/show_bug.cgi?id=22803
+template  __attribute__((always_inline))
+inline _LIBCPP_INLINE_VISIBILITY T

* `T` and `increment` need to be reserved names.
* Never use `__attribute__((always_inline))` directly, that's why we have 
visibility macros.



Comment at: libcxx/include/memory:3802
+{
+  return __libcpp_atomic_add(, 1, _AO_Relaxed);
+}

Why add `increment` and `decrement` at all? Just manually inline 
`__libcpp_atomic_add` at the callsites.



Comment at: libcxx/include/memory:3818
 virtual ~bad_weak_ptr() _NOEXCEPT;
-virtual const char* what() const  _NOEXCEPT;
+virtual const char* what() const  _NOEXCEPT {
+  return "bad_weak_ptr";

Why would you  want to inline this?


Repository:
  rL LLVM

https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Marshall Clow via cfe-commits]

mclow.lists added a comment.

How does this play with existing binaries?  Applications that expect these 
functions to exist in the dylib?


Repository:
  rL LLVM

https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kevin Hu via cfe-commits]

hxy9243 updated this revision to Diff 73293.
hxy9243 added a comment.

Addresses comments from @halyavin, rename "atomic_support.h" to 
"__atomic_support" to avoid collisions with application headers.


Repository:
  rL LLVM

https://reviews.llvm.org/D24991

Files:
  libcxx/include/__atomic_support
  libcxx/include/memory
  libcxx/src/include/atomic_support.h
  libcxx/src/memory.cpp
  libcxx/src/mutex.cpp

Index: libcxx/src/mutex.cpp
===
--- libcxx/src/mutex.cpp
+++ libcxx/src/mutex.cpp
@@ -12,7 +12,7 @@
 #include "limits"
 #include "system_error"
 #include "cassert"
-#include "include/atomic_support.h"
+#include "__atomic_support"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 #ifndef _LIBCPP_HAS_NO_THREADS
Index: libcxx/src/memory.cpp
===
--- libcxx/src/memory.cpp
+++ libcxx/src/memory.cpp
@@ -13,85 +13,17 @@
 #include "mutex"
 #include "thread"
 #endif
-#include "include/atomic_support.h"
+#include "__atomic_support"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-namespace
-{
-
-// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
-// should be sufficient for thread safety.
-// See https://llvm.org/bugs/show_bug.cgi?id=22803
-template 
-inline T
-increment(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, 1, _AO_Relaxed);
-}
-
-template 
-inline T
-decrement(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, -1, _AO_Acq_Rel);
-}
-
-}  // namespace
-
 const allocator_arg_t allocator_arg = allocator_arg_t();
 
 bad_weak_ptr::~bad_weak_ptr() _NOEXCEPT {}
 
-const char*
-bad_weak_ptr::what() const _NOEXCEPT
-{
-return "bad_weak_ptr";
-}
+__shared_count::~__shared_count() {}
 
-__shared_count::~__shared_count()
-{
-}
-
-void
-__shared_count::__add_shared() _NOEXCEPT
-{
-increment(__shared_owners_);
-}
-
-bool
-__shared_count::__release_shared() _NOEXCEPT
-{
-if (decrement(__shared_owners_) == -1)
-{
-__on_zero_shared();
-return true;
-}
-return false;
-}
-
-__shared_weak_count::~__shared_weak_count()
-{
-}
-
-void
-__shared_weak_count::__add_shared() _NOEXCEPT
-{
-__shared_count::__add_shared();
-}
-
-void
-__shared_weak_count::__add_weak() _NOEXCEPT
-{
-increment(__shared_weak_owners_);
-}
-
-void
-__shared_weak_count::__release_shared() _NOEXCEPT
-{
-if (__shared_count::__release_shared())
-__release_weak();
-}
+__shared_weak_count::~__shared_weak_count() {}
 
 void
 __shared_weak_count::__release_weak() _NOEXCEPT
Index: libcxx/src/include/atomic_support.h
===
--- libcxx/src/include/atomic_support.h
+++ /dev/null
@@ -1,158 +0,0 @@
-//===--===
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is dual licensed under the MIT and the University of Illinois Open
-// Source Licenses. See LICENSE.TXT for details.
-//
-//===--===
-
-#ifndef ATOMIC_SUPPORT_H
-#define ATOMIC_SUPPORT_H
-
-#include "__config"
-#include "memory" // for __libcpp_relaxed_load
-
-#if defined(__clang__) && __has_builtin(__atomic_load_n) \
-   && __has_builtin(__atomic_store_n)\
-   && __has_builtin(__atomic_add_fetch)  \
-   && __has_builtin(__atomic_compare_exchange_n) \
-   && defined(__ATOMIC_RELAXED)  \
-   && defined(__ATOMIC_CONSUME)  \
-   && defined(__ATOMIC_ACQUIRE)  \
-   && defined(__ATOMIC_RELEASE)  \
-   && defined(__ATOMIC_ACQ_REL)  \
-   && defined(__ATOMIC_SEQ_CST)
-#   define _LIBCPP_HAS_ATOMIC_BUILTINS
-#elif !defined(__clang__) && defined(_GNUC_VER) && _GNUC_VER >= 407
-#   define _LIBCPP_HAS_ATOMIC_BUILTINS
-#endif
-
-#if !defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
-# if defined(_MSC_VER) && !defined(__clang__)
-_LIBCPP_WARNING("Building libc++ without __atomic builtins is unsupported")
-# else
-#   warning Building libc++ without __atomic builtins is unsupported
-# endif
-#endif
-
-_LIBCPP_BEGIN_NAMESPACE_STD
-
-namespace {
-
-#if defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
-
-enum __libcpp_atomic_order {
-_AO_Relaxed = __ATOMIC_RELAXED,
-_AO_Consume = __ATOMIC_CONSUME,
-_AO_Acquire = __ATOMIC_ACQUIRE,
-_AO_Release = __ATOMIC_RELEASE,
-_AO_Acq_Rel = __ATOMIC_ACQ_REL,
-_AO_Seq = __ATOMIC_SEQ_CST
-};
-
-template 
-inline _LIBCPP_INLINE_VISIBILITY
-void __libcpp_atomic_store(_ValueType* __dest, _FromType __val,
-   int __order = _AO_Seq)
-{
-__atomic_store_n(__dest, __val, __order);
-}
-
-template 

Re: [PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Andrey Khalyavin via cfe-commits]

halyavin added a subscriber: halyavin.


Comment at: libcxx/include/atomic_support.h:1
@@ +1,2 @@
+//===--===
+//

Non-standard include files in the main include directory must start with __ to 
avoid collisions with application headers.


Comment at: libcxx/include/atomic_support.h:1
@@ +1,2 @@
+//===--===
+//

halyavin wrote:
> Non-standard include files in the main include directory must start with __ 
> to avoid collisions with application headers.
Does anyone know why this header exists and atomic header can't be used 
instead? 


Repository:
  rL LLVM

https://reviews.llvm.org/D24991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Kevin Hu via cfe-commits]

hxy9243 created this revision.
hxy9243 added reviewers: sebpop, hiraditya, wmi.
hxy9243 added a subscriber: cfe-commits.
hxy9243 set the repository for this revision to rL LLVM.

This patch moves some existing functions from the memory.cpp to the memory 
header file, so that they could be properly inlined, which gives potential 
optimization opportunities and performance benefits.

Repository:
  rL LLVM

https://reviews.llvm.org/D24991

Files:
  libcxx/include/atomic_support.h
  libcxx/include/memory
  libcxx/src/include/atomic_support.h
  libcxx/src/memory.cpp
  libcxx/src/mutex.cpp

Index: libcxx/src/mutex.cpp
===
--- libcxx/src/mutex.cpp
+++ libcxx/src/mutex.cpp
@@ -12,7 +12,7 @@
 #include "limits"
 #include "system_error"
 #include "cassert"
-#include "include/atomic_support.h"
+#include "atomic_support.h"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 #ifndef _LIBCPP_HAS_NO_THREADS
Index: libcxx/src/memory.cpp
===
--- libcxx/src/memory.cpp
+++ libcxx/src/memory.cpp
@@ -13,85 +13,17 @@
 #include "mutex"
 #include "thread"
 #endif
-#include "include/atomic_support.h"
+#include "atomic_support.h"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-namespace
-{
-
-// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
-// should be sufficient for thread safety.
-// See https://llvm.org/bugs/show_bug.cgi?id=22803
-template 
-inline T
-increment(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, 1, _AO_Relaxed);
-}
-
-template 
-inline T
-decrement(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, -1, _AO_Acq_Rel);
-}
-
-}  // namespace
-
 const allocator_arg_t allocator_arg = allocator_arg_t();
 
 bad_weak_ptr::~bad_weak_ptr() _NOEXCEPT {}
 
-const char*
-bad_weak_ptr::what() const _NOEXCEPT
-{
-return "bad_weak_ptr";
-}
+__shared_count::~__shared_count() {}
 
-__shared_count::~__shared_count()
-{
-}
-
-void
-__shared_count::__add_shared() _NOEXCEPT
-{
-increment(__shared_owners_);
-}
-
-bool
-__shared_count::__release_shared() _NOEXCEPT
-{
-if (decrement(__shared_owners_) == -1)
-{
-__on_zero_shared();
-return true;
-}
-return false;
-}
-
-__shared_weak_count::~__shared_weak_count()
-{
-}
-
-void
-__shared_weak_count::__add_shared() _NOEXCEPT
-{
-__shared_count::__add_shared();
-}
-
-void
-__shared_weak_count::__add_weak() _NOEXCEPT
-{
-increment(__shared_weak_owners_);
-}
-
-void
-__shared_weak_count::__release_shared() _NOEXCEPT
-{
-if (__shared_count::__release_shared())
-__release_weak();
-}
+__shared_weak_count::~__shared_weak_count() {}
 
 void
 __shared_weak_count::__release_weak() _NOEXCEPT
Index: libcxx/src/include/atomic_support.h
===
--- libcxx/src/include/atomic_support.h
+++ /dev/null
@@ -1,158 +0,0 @@
-//===--===
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is dual licensed under the MIT and the University of Illinois Open
-// Source Licenses. See LICENSE.TXT for details.
-//
-//===--===
-
-#ifndef ATOMIC_SUPPORT_H
-#define ATOMIC_SUPPORT_H
-
-#include "__config"
-#include "memory" // for __libcpp_relaxed_load
-
-#if defined(__clang__) && __has_builtin(__atomic_load_n) \
-   && __has_builtin(__atomic_store_n)\
-   && __has_builtin(__atomic_add_fetch)  \
-   && __has_builtin(__atomic_compare_exchange_n) \
-   && defined(__ATOMIC_RELAXED)  \
-   && defined(__ATOMIC_CONSUME)  \
-   && defined(__ATOMIC_ACQUIRE)  \
-   && defined(__ATOMIC_RELEASE)  \
-   && defined(__ATOMIC_ACQ_REL)  \
-   && defined(__ATOMIC_SEQ_CST)
-#   define _LIBCPP_HAS_ATOMIC_BUILTINS
-#elif !defined(__clang__) && defined(_GNUC_VER) && _GNUC_VER >= 407
-#   define _LIBCPP_HAS_ATOMIC_BUILTINS
-#endif
-
-#if !defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
-# if defined(_MSC_VER) && !defined(__clang__)
-_LIBCPP_WARNING("Building libc++ without __atomic builtins is unsupported")
-# else
-#   warning Building libc++ without __atomic builtins is unsupported
-# endif
-#endif
-
-_LIBCPP_BEGIN_NAMESPACE_STD
-
-namespace {
-
-#if defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
-
-enum __libcpp_atomic_order {
-_AO_Relaxed = __ATOMIC_RELAXED,
-_AO_Consume = __ATOMIC_CONSUME,
-_AO_Acquire = __ATOMIC_ACQUIRE,
-_AO_Release = __ATOMIC_RELEASE,
-_AO_Acq_Rel = __ATOMIC_ACQ_REL,
-_AO_Seq = __ATOMIC_SEQ_CST
-};
-
-template 
-inline