EricWF created this revision. EricWF added reviewers: mclow.lists, laxmansole, hiraditya, dblaikie. EricWF added a subscriber: cfe-commits.
r280944 attempted to add both `inline` and `_LIBCPP_INLINE_VISIBILITY` to basic_string's destructor in order to achieve better inlining behavior. Unfortunately applying `_LIBCPP_INLINE_VISIBILITY` to the destructor [triggers a Clang bug](http://llvm.org/PR30341). This patch attempts to still get better inlining behavior while working around the Clang bug by simply applying `inline` without the attribute. Unfortunately applying only `inline` causes the destructor to be inlined at -O2 or greater whereas the attribute results in inlining at all optimization levels. However this is still better than no inlining at all. See https://godbolt.org/g/8UbKwb for example assembly. My only concern with this patch is marking the function as `inline` without making the visibility hidden. I don't think that should have any weird consequences but I'm not 100%. https://reviews.llvm.org/D24599 Files: include/string Index: include/string =================================================================== --- include/string +++ include/string @@ -1798,6 +1798,7 @@ #endif // _LIBCPP_HAS_NO_GENERALIZED_INITIALIZERS template <class _CharT, class _Traits, class _Allocator> +inline // _LIBCPP_INLINE_VISIBILITY FIXME: http://llvm.org/PR30341 basic_string<_CharT, _Traits, _Allocator>::~basic_string() { #if _LIBCPP_DEBUG_LEVEL >= 2
Index: include/string =================================================================== --- include/string +++ include/string @@ -1798,6 +1798,7 @@ #endif // _LIBCPP_HAS_NO_GENERALIZED_INITIALIZERS template <class _CharT, class _Traits, class _Allocator> +inline // _LIBCPP_INLINE_VISIBILITY FIXME: http://llvm.org/PR30341 basic_string<_CharT, _Traits, _Allocator>::~basic_string() { #if _LIBCPP_DEBUG_LEVEL >= 2
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
