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

Eric Fiselier via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Jan 14 03:22:22 PST 2017


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

Almost LGTM. Just a couple of inline comments left. Thanks for working on this!



================
Comment at: libcxx/include/memory:3691
+                       && __has_builtin(__atomic_compare_exchange_n) \
+                       && defined(__ATOMIC_RELAXED)                  \
+                       && defined(__ATOMIC_CONSUME)                  \
----------------
1. I would reduce these checks down to only what we need in the headers.
2. I would rename `_LIBCPP_HAS_ATOMIC_BUILTINS` to `_LIBCPP_HAS_BUILTIN_ATOMIC_SUPPORT` so it doesn't conflict with `atomic_support.h` and so we don't get it confused with all of the other `_LIBCPP_ATOMIC` configuration macros.
3.  This is missing the configuration checks for GCC. Specifically `#elif !defined(__clang__) && defined(_GNUC_VER) && _GNUC_VER >= 407`


================
Comment at: libcxx/include/memory:3701
+template <class T>
+inline T
+__libcpp_atomic_refcount_increment(T& t) _NOEXCEPT
----------------
`inline _LIBCPP_INLINE_VISIBILITY`


================
Comment at: libcxx/include/memory:3759
+#ifdef _LIBCPP_BUILDING_MEMORY
+    void _LIBCPP_FUNC_VIS __add_shared() _NOEXCEPT;
+    bool _LIBCPP_FUNC_VIS __release_shared() _NOEXCEPT;
----------------
`_LIBCPP_FUNC_VIS` goes before the return type.


================
Comment at: libcxx/include/memory:3797
+    void _LIBCPP_FUNC_VIS __add_shared() _NOEXCEPT;
+    void _LIBCPP_FUNC_VIS __add_weak() _NOEXCEPT;
+    void _LIBCPP_FUNC_VIS __release_shared() _NOEXCEPT;
----------------
`_LIBCPP_FUNC_VIS` goes before the return type.


Repository:
  rL LLVM

https://reviews.llvm.org/D24991





More information about the cfe-commits mailing list